Skip to content

docs(openworlds): add fidelity and asset contract - #124

Merged
100yenadmin merged 2 commits into
mainfrom
docs/openworlds-fidelity-contract
May 25, 2026
Merged

docs(openworlds): add fidelity and asset contract#124
100yenadmin merged 2 commits into
mainfrom
docs/openworlds-fidelity-contract

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented May 25, 2026

Copy link
Copy Markdown
Member

Summary

Adds the fidelity-first contract for integrating the OpenWorlds design into ClawDnD after PR #123 proved that a hand SwiftUI repaint is the wrong path.

This PR is intentionally docs-only. It defines:

  • the primary OpenWorlds visual/reference source files
  • which screenshots/uploads are reference-only and must not be committed
  • which prototype files require rewrite or removal before production use
  • the viewer-hosted /openworlds/ architecture and engine-state boundary
  • the PR sequence for exact surface, native host, launcher binding, and later gameplay surfaces
  • visual validation gates and docs-only CI expectations

Refs #113.
Refs #122.
Supersedes the implementation direction in #123, but does not close it yet. #123 should remain draft/unmerged until the viewer-hosted OpenWorlds replacement is open.

Why

The OpenWorlds export depends on the actual web implementation for its fidelity: CSS texture layers, parchment/walnut/brass treatment, typography, SVG ornaments, dense layout, and interaction states. Recreating that by hand in SwiftUI lost the design. This contract locks the safer direction before code/assets move into the repo.

Boundaries

  • No engine, rules, voice, content, or QA narrative changes.
  • No OpenWorlds screenshots, uploaded reference images, private state, transcripts, or zip bundles committed.
  • Browser surfaces remain read-only except for the existing /move player-intent path.
  • Live CDN dependencies are banned for packaged app work; local vendored runtime is allowed only for the first exact-fidelity sprint with notices.

Validation

Local validation run from /Volumes/LEXAR/repos/ClawDnD-openworlds-contract:

git diff --check
python3 -m py_compile viewer/server.py
python3 -m unittest discover -s viewer/tests -q
python3 scripts/license_check.py

Results:

  • viewer/server.py py_compile passed
  • viewer tests passed: 14 tests
  • license check passed
  • diff check passed

Adversarial review:

  • Architecture/state-authority reviewer approved after confirming engine sole-writer and /move boundaries.
  • Asset/license reviewer found overbroad copy language; fixed by requiring audited cleaned bundles, provenance, notices, and explicit prototype data handling.
  • Test/CI reviewer found docs-only Swift CI overclaim and imprecise feat(macos): add OpenWorlds product shell #123 disposition; fixed in the fidelity plan.

Next PR

PR B should create a cleaned, audited viewer/openworlds/ bundle served by viewer/server.py under /openworlds/, with local runtime dependencies and no reference images.

Summary by CodeRabbit

  • Documentation
    • Added a design asset policy covering provenance, allowed/blocked asset types, and guidelines for prototype vs production usage.
    • Added an implementation fidelity plan describing the staged rollout, responsibilities between native and web surfaces, constraints on browser-side data mutations, and required validation/checklist steps before release.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR adds two OpenWorlds docs: an asset-provenance and repo-content policy, and a fidelity rollout plan for the exact web surface with staged integration and validation requirements.

Changes

OpenWorlds Integration

Layer / File(s) Summary
Asset provenance and repo content policy
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md
Defines provenance buckets (primary contract, prototype/demo, production-excluded, secondary, reference-only), enumerates allowed vs. prohibited repo artifacts (OpenWorlds.zip, unprovenanced screenshots, private/runtime prototypes), specifies prototype dependency constraints (no CDN at runtime; temporary local vendoring allowed; bundling for release), and adds a visual-work PR checklist and default PR statement.
Web surface rollout plan and architecture
docs/OPENWORLDS_FIDELITY_PLAN.md
Documents the exact web-surface rollout: architecture split (macOS SwiftUI supervision vs viewer-hosted /openworlds/), browser write restrictions, staged rollout steps (asset/contract work, cleaned same-origin bundle, native launch behavior, read-model binding, read-only gameplay surfaces before actions), visual parity gates, PR #123 disposition, and local/CI validation/licensing checks.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related issues

  • #122: These docs implement the asset-policy and fidelity-plan auditing and PR checklist requirements described in that issue.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the changeset: two docs files added for OpenWorlds fidelity and asset policy/contract.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • LINEAR integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md`:
- Around line 12-19: The file OPENWORLDS_DESIGN_ASSET_POLICY.md currently
contains host-specific absolute paths (e.g., entries like
`/Volumes/LEXAR/Codex/openworlds-design-2026-05-25/openworlds/Open Worlds.html`,
`/Volumes/LEXAR/.../styles.css`, `/Volumes/LEXAR/.../app.jsx`,
`/Volumes/LEXAR/.../chrome.jsx`, `/Volumes/LEXAR/.../screen-*.jsx`,
`/Volumes/LEXAR/.../camp-sidebar.jsx`, `/Volumes/LEXAR/.../toast.jsx`,
`/Volumes/LEXAR/.../tooltip.jsx`) that leak personal/machine info; replace each
absolute path with a repo-stable provenance reference: use a neutral artifact
identifier (artifact name), a content hash (SHA256 or similar), acquisition
date, and a repo-relative pointer or canonical storage location (e.g.,
artifacts/ or docs/ path), and remove any /Users or /Volumes segments; update
all similar occurrences (lines noted in the comment: 23-24, 27-33, 37-40, and
the referenced line 39) so every entry follows the artifact-name + hash +
acquisition-date + repo-relative path pattern.

In `@docs/OPENWORLDS_FIDELITY_PLAN.md`:
- Line 84: The Markdown line contains a bare "`#123`" which is being parsed as an
ATX heading; fix by either escaping the hash or moving it inline so it isn't
interpreted as a heading—for example replace the standalone "`#123` linking to the
fidelity contract PR..." with "\`#123` linking to the fidelity contract PR..." or
"linking to the fidelity contract PR (see `#123`)" so the token is treated as
plain text; update the line containing "`#123`" accordingly in
OPENWORLDS_FIDELITY_PLAN.md.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 422df6a3-8d0c-4788-9758-cd68748c8b65

📥 Commits

Reviewing files that changed from the base of the PR and between ed64382 and 093b1d3.

📒 Files selected for processing (2)
  • docs/OPENWORLDS_DESIGN_ASSET_POLICY.md
  • docs/OPENWORLDS_FIDELITY_PLAN.md
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
docs/OPENWORLDS_FIDELITY_PLAN.md

[warning] 84-84: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🔇 Additional comments (2)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (1)

41-99: LGTM!

docs/OPENWORLDS_FIDELITY_PLAN.md (1)

1-83: LGTM!

Also applies to: 85-123

Comment thread docs/OPENWORLDS_DESIGN_ASSET_POLICY.md Outdated
Comment thread docs/OPENWORLDS_FIDELITY_PLAN.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (1)

79-79: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Point the policy at the actual browser-side enforcement for campaign mutation

  • Enforcement exists: viewer/server.py makes the viewer read-only by only allowing writes via POST /move, which (1) is disabled when $CLAWDND_PLAYER_MOVES is unset, (2) appends sanitized player move intents to the $CLAWDND_PLAYER_MOVES JSONL sink (never campaign state), and (3) rejects moves targeting a non-live/other campaign id; sanitize_move() also whitelists kind and forces role: "player".
  • The UI mirrors it: viewer/dashboard.html gates the action layer on state.live and only sends moves through fetch("/move") (no direct state writes).

Update docs/OPENWORLDS_DESIGN_ASSET_POLICY.md around line 79 to explicitly name/link this enforcement mechanism (and optionally the related contract test: servers/engine/tests/test_provider_contract_smoke.py).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md` at line 79, Update the policy text to
explicitly point at the browser-side enforcement implemented in viewer/server.py
(the POST /move endpoint) and the UI gating in viewer/dashboard.html: mention
that viewer/server.py makes the viewer read-only by only accepting sanitized
player intents via POST /move (see sanitize_move() which whitelists kind and
forces role:"player", appends to $CLAWDND_PLAYER_MOVES JSONL sink, and rejects
moves for non-live/other campaign ids and when $CLAWDND_PLAYER_MOVES is unset),
and note that viewer/dashboard.html only sends moves via fetch("/move") and
gates the action layer on state.live; optionally reference the related contract
test servers/engine/tests/test_provider_contract_smoke.py.
docs/OPENWORLDS_FIDELITY_PLAN.md (2)

27-37: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Confirm viewer/server.py enforces the browser write boundary

  • do_POST only permits POST /move and POST /speak; anything else 404s.
  • POST /move writes only to the append-only $CLAWDND_PLAYER_MOVES sink (via _moves_path()), and returns a “read-only (no live game)” error without writing when that env var/target isn’t available or writable.
  • Moves are constrained by sanitize_move(...) and also refused when a campaign tag doesn’t match the attached live campaign (#49).
  • No request handler path in viewer/server.py writes snapshot.json, play-state, or qa/state (those are used as read-only inputs for rendering/monitoring).

Add a small integration test to assert that POST /move is the only endpoint that can create/append the moves log, and that attempts to hit other routes don’t change snapshot.json / play-state / qa/state.


123-123: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Clarify "read-only adversarial agents" terminology.

Line 123 mentions "read-only adversarial agents" which may be unclear to future contributors.

Consider clarifying:

  • Is this referring to automated testing tools?
  • Is it referring to AI code review agents (like CodeRabbit)?
  • Or is it a security review process?

Example clarification:

-Use CodeRabbit and read-only adversarial agents before merge.
+Use CodeRabbit and read-only adversarial review agents (automated security/quality scanners) before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/OPENWORLDS_FIDELITY_PLAN.md` at line 123, The phrase "read-only
adversarial agents" is ambiguous; update the sentence to explicitly define what
you mean (e.g., automated testing/security scanners, AI code-review bots like
CodeRabbit, or a manual security review) and give one or two examples and
intent; for example replace or augment the phrase with "read-only adversarial
agents (e.g., automated security scanners or AI review bots such as CodeRabbit)
used to probe for vulnerabilities without modifying code" and note whether they
run automatically in CI, are human-operated, or are merely advisory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md`:
- Line 79: Update the policy text to explicitly point at the browser-side
enforcement implemented in viewer/server.py (the POST /move endpoint) and the UI
gating in viewer/dashboard.html: mention that viewer/server.py makes the viewer
read-only by only accepting sanitized player intents via POST /move (see
sanitize_move() which whitelists kind and forces role:"player", appends to
$CLAWDND_PLAYER_MOVES JSONL sink, and rejects moves for non-live/other campaign
ids and when $CLAWDND_PLAYER_MOVES is unset), and note that
viewer/dashboard.html only sends moves via fetch("/move") and gates the action
layer on state.live; optionally reference the related contract test
servers/engine/tests/test_provider_contract_smoke.py.

In `@docs/OPENWORLDS_FIDELITY_PLAN.md`:
- Line 123: The phrase "read-only adversarial agents" is ambiguous; update the
sentence to explicitly define what you mean (e.g., automated testing/security
scanners, AI code-review bots like CodeRabbit, or a manual security review) and
give one or two examples and intent; for example replace or augment the phrase
with "read-only adversarial agents (e.g., automated security scanners or AI
review bots such as CodeRabbit) used to probe for vulnerabilities without
modifying code" and note whether they run automatically in CI, are
human-operated, or are merely advisory.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 31dd7ab6-95c7-4fc4-b2e3-f162fc9a5f11

📥 Commits

Reviewing files that changed from the base of the PR and between 093b1d3 and cbe6324.

📒 Files selected for processing (2)
  • docs/OPENWORLDS_DESIGN_ASSET_POLICY.md
  • docs/OPENWORLDS_FIDELITY_PLAN.md
📜 Review details
🔇 Additional comments (10)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (5)

10-18: LGTM!


19-51: LGTM!


53-56: LGTM!


57-78: LGTM!


81-112: LGTM!

docs/OPENWORLDS_FIDELITY_PLAN.md (5)

1-25: LGTM!


39-81: LGTM!


82-90: LGTM!


92-103: LGTM!


109-118: ⚡ Quick win

Remove the concern about missing validation script

  • script/build_and_run.sh exists in the repo and is executable.
  • The script’s usage/case handling includes --verify|verify, so ./script/build_and_run.sh --verify is a valid invocation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant