feat(viewer): bind OpenWorlds campaigns to local state - #127
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used🪛 Ruff (0.15.14)viewer/server.py[warning] 817-817: Using the global statement to update (PLW0603) 🔇 Additional comments (9)
📝 WalkthroughWalkthroughBackend discovers and summarizes campaigns from viewer state plus repo play-state/qa, caches and serves a capped catalog via GET /openworlds/campaigns.json. Frontend loads the catalog, keeps selection synced, supports resume/monitor redirects, and renders derived display fields. Tests validate projection, privacy, and discovery roots. ChangesCampaign Catalog Backend
Campaign Browser Frontend
Campaign Catalog Tests
Possibly Related Issues
Possibly Related PRs
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@viewer/openworlds/screen-launcher.jsx`:
- Around line 8-11: The effect and resume paths currently reuse
state.activeCampaign without confirming that id exists in campaigns, letting
selected become a stale invalid id; update the React.useEffect that sets
selected so it first checks campaigns.some(c => c.id === state?.activeCampaign)
and only uses state.activeCampaign when that check passes, otherwise fall back
to campaigns[0]?.id || ""; apply the same membership validation wherever
selected is re-assigned (the other effect at lines 13-17) and ensure onResume
uses a validated id (i.e., only resume when campaigns.some(c => c.id ===
selected) is true or substitute a valid campaign id) so selection and resume
always target an existing campaign.
- Around line 251-253: campaignRegion currently returns raw values which may be
non-strings and cause downstream calls like region.toLowerCase() to throw;
update the campaignRegion function to coerce the chosen value to a string (or
fall back to "Unknown") before returning — e.g. pick the first defined value
from c?.region, c?.location, c?.world and return String(value) (or value != null
? String(value) : "Unknown") so callers can safely call string methods.
In `@viewer/server.py`:
- Around line 751-792: The _openworlds_campaigns function rebuilds the entire
catalog on every call; add an mtime-based cache (pattern used by
_monitor_card_cache and _monitor_campaigns) to avoid reloading unchanged
campaign snapshot.json files on frequent polls: compute a cache key from
current_campaigns_dir (or roots) plus the latest mtime of snapshot.json files
(or per-root mtimes), store the summary list and total/now in a module-level
cache with that mtime, and return the cached value when mtimes haven’t changed;
update cache on change and ensure cache eviction/size (80) semantics match
existing behavior.
In `@viewer/tests/test_openworlds_static.py`:
- Around line 197-201: Add explicit assertions for the readOnly flag on the two
catalog entries: assert that by_id["play:play-20260525:camp_play"]["readOnly"]
is False (repo-local entry) and that by_id["qa:wave3-red:camp_qa"]["readOnly"]
is True (QA catalog entry); place these assertions alongside the existing
provider/canResume/monitorUrl assertions in test_openworlds_static.py so the
test locks the readOnly contract for both sources.
- Around line 155-158: Test currently checks substrings in the serialized JSON;
instead, parse the JSON back to a Python object and assert that private keys
themselves are not present anywhere in the structure. After the existing encoded
= json.dumps(campaign), call json.loads(encoded) and run a small recursive check
(e.g., assert_no_private_keys) over the resulting dict/list to ensure keys like
"private note", "hidden agenda", "private canon" (and other sensitive keys such
as "notes", "scenes", "lore") do not appear as dictionary keys anywhere; update
the test in test_openworlds_static.py (around the existing encoded variable) to
use this parsed-object key-based assertion rather than substring assertions.
🪄 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: 3500984f-0ad2-4942-8b21-4cedfd7137e9
📒 Files selected for processing (4)
viewer/openworlds/app.jsxviewer/openworlds/screen-launcher.jsxviewer/server.pyviewer/tests/test_openworlds_static.py
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (3)
viewer/openworlds/screen-launcher.jsx (1)
viewer/openworlds/chrome.jsx (2)
Placeholder(204-213)Pill(200-202)
viewer/tests/test_openworlds_static.py (1)
viewer/tests/test_build_options_bridge.py (2)
setUp(17-20)tearDown(22-26)
viewer/server.py (1)
viewer/tests/test_openworlds_static.py (1)
_Handler(19-19)
🔇 Additional comments (16)
viewer/server.py (13)
47-47: LGTM!
533-537: LGTM!
540-544: LGTM!
547-557: LGTM!
560-600: LGTM!
603-607: LGTM!
610-615: LGTM!
618-644: LGTM!
647-662: LGTM!
665-674: LGTM!
677-748: LGTM!
1682-1699: LGTM!
1846-1847: LGTM!viewer/openworlds/app.jsx (1)
21-65: LGTM!viewer/openworlds/screen-launcher.jsx (1)
18-25: LGTM!Also applies to: 122-126, 141-147, 192-196, 206-207, 217-217, 238-249, 255-258, 260-264, 280-281, 309-311
viewer/tests/test_openworlds_static.py (1)
28-29: LGTM!Also applies to: 47-47, 83-85, 101-154, 160-196, 203-206
… (#205) Generalize the proven companion stage-machine onto a FACTION-owned reputation/standing gauge so a faction questline can be JOINED, grown through service, and led to a world-changing finale — closing the map's "Faction.reputation is tracked but nothing reads it to gate anything" gap. Reuse, not rebuild: the lifecycle enum, the _apply_structured_effect ripple path, the degrade-not-abort seed loader, and the advise-not-act Director surface are all generalized from the companion-arc / Event (Layer 3) machinery. - models: additive Faction fields (rank, monotonic standing>=0, joined, questline_arc_id; old snapshots round-trip) + FactionArc/FactionArcStage (per-stage unlock_at gauge gate, Outcome-shaped finale_effect with an effect_applied idempotency latch) + Campaign.faction_arcs. - faction_arc.py (pure module, mirrors events.py): gauge-gate eval (locked below unlock_at, available at/above; sign picks direction), join-gated evaluate, read-only rank-up detector, and a finale that ripples via worldsim._apply_structured_effect exactly once. - server tools: join_faction (arms the arc), grant_standing (the monotonic membership gauge), advance_faction_arc (gate-enforced, finale-once), set_faction_arc, get_faction_arcs, check_faction_arcs (advance + advisory nudges) — mirroring the companion-quest-arc surface. - scene_debt: a faction_rank_available detector (advisory, severity low). - content: faction seed reads the new fields; a new degrade-not-abort faction_arcs world/overlay block (its own block, since a faction isn't a Character so companion_seeds doesn't fit). - exemplar: the Flaming Fist's "Banner of the Fist" arc in world.json — take the oath (reputation) -> earn the captaincy (standing) -> raise the banner (the world-changing finale puts the Gate under the Fist's banner). Original in-character prose, CC-BY fan content. Additive + idempotent (empty == today byte-for-byte). 39 new tests; full engine suite 1253 passing single-process; license_check green. Co-authored-by: Eva <eva@100yen.org>
Summary
Binds the exact OpenWorlds Chronicles launcher to real local ClawDnD campaign state instead of prototype campaign rows.
GET /openworlds/campaigns.jsonviewer API.play-state/*, and repo-localqa/state/*campaign snapshots.Refs #114.
Architecture Notes
The new viewer catalog is deliberately downstream-only:
viewer/server.py_campaign_catalog_roots()discovers read-only roots from the activeCLAWDND_STATE_DIR,play-state/*, andqa/state/*.build_openworlds_campaign_summary(...)projects only player-facing fields and omits local absolute paths,dm_notes, lore recall input, sealed agenda text, raw transcripts, and other private state._openworlds_campaigns(...)returns the browser contract plusstate_authority: engineandwrite_lane: /move.canResumeis true only when the campaign is in the viewer's active state root, because/dashboard?campaign=...cannot switch the server to a different state dir.viewer/openworlds/app.jsx/openworlds/campaigns.jsononce on load.viewer/openworlds/screen-launcher.jsxState Authority / Safety
This PR does not change engine, rules, voice, provider, or macOS public APIs.
The browser still has no direct campaign write path. The only game-intent write lane remains
POST /move, and this PR does not add new POST routes.Validation
Local validation from
/Volumes/LEXAR/repos/ClawDnD-openworlds-campaigns-binding:Result:
20viewer tests passed; license check passed.Rendered smoke:
viewer/server.pyagainst a throwaway state dir under/Volumes/LEXAR/Codex/clawdnd-openworlds-campaign-smoke.*./openworlds/campaigns.jsonreturned the real campaign row.http://127.0.0.1:8876/openworlds/.Smoke at the GateandLower City, excludes the prototypeThe Long Road to Odrun, and reports no runtime exceptions.No Swift files changed, so
swift build --package-path macos/ClawDnDAppwas intentionally not rerun for this viewer-only PR.Rollback
Reverting this commit returns OpenWorlds to the exact prototype-data launcher while preserving the already-merged
/openworlds/static route and macOS WebView route.Summary by CodeRabbit
New Features
Bug Fixes
Tests