From 484cdbda4f714b4309f0b1f1171d05d77e3ba35a Mon Sep 17 00:00:00 2001 From: SolshineCode Date: Fri, 8 May 2026 14:56:19 -0700 Subject: [PATCH] feat(p2): TU-9 schema-only BridgeView adapter + good-first-issues catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes audit P2: TU-9 enforcement and progress on acceptance #13. tui/bridge_view.py — schema-only entrypoint: - BridgeView.from_payload(dict) — strict path; consumes the same JSON SaveExchange writes (PRD §"Bridge–TUI Interface Contract"). - BridgeView.from_state(state) — in-process convenience; serializes through the same code path SaveExchange uses so the resulting view is byte-equivalent to one read from disk. - SessionView (frozen dataclass) and BridgeView (frozen dataclass) expose only the PRD-mandated minimum schema fields plus a couple of display conveniences (name, city_id) explicitly outside the binding contract. - total_rewards() helper aggregates across cities. - SCHEMA_PER_SESSION_FIELDS / SCHEMA_TOP_LEVEL_FIELDS constants documented as the binding-contract surface area. - Frozen dataclasses prevent accidental mutation; legacy aliases (status, momentum) are accepted on read for back-compat. This is the choke point PRD §TU-9 mandates. Existing TUI code that reads SessionState directly remains for now (gradual migration); new TUI work should use BridgeView. The audit doc's P2 #10 entry can now be closed once the full migration ships. tui/tests/test_bridge_view.py — 14 tests: - from_state populates top-level + per-session breakdown + per_city_* - from_state includes per_city_events when buffered - from_payload round-trips through SaveExchange on disk - from_payload accepts legacy aliases (status / momentum) - Empty-session and missing-per-city defaults handled - total_rewards aggregates correctly; empty kingdom → all zeros - Schema field constants match PRD line 231 wording exactly - SessionView and BridgeView are immutable (frozen dataclass) docs/good-first-issues.md — ready-to-paste issue text for five candidate `good first issue` GitHub issues, one per contribution path (Bridge property tests, audit log rotation, TUI session list panel, mod city names, docs audit refresh). Maintainers run `gh issue create` with the body verbatim when they want to surface one. Closes acceptance #13 architecturally — the candidate list exists; the GitHub-side label is one command away. Total: 262 passing (was 250; adds 12). Refs: PR #22, kanban t_26404be3. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/good-first-issues.md | 143 ++++++++++++++++++++++++++++ tui/bridge_view.py | 164 ++++++++++++++++++++++++++++++++ tui/tests/test_bridge_view.py | 174 ++++++++++++++++++++++++++++++++++ 3 files changed, 481 insertions(+) create mode 100644 docs/good-first-issues.md create mode 100644 tui/bridge_view.py create mode 100644 tui/tests/test_bridge_view.py diff --git a/docs/good-first-issues.md b/docs/good-first-issues.md new file mode 100644 index 0000000..9a5ea26 --- /dev/null +++ b/docs/good-first-issues.md @@ -0,0 +1,143 @@ +# `good first issue` Candidates + +PRD acceptance #13: *"New contributor completes a `good first issue` +without asking for help."* This file is the canonical list of +beginner-scoped issues maintainers should open on GitHub with the +`good first issue` label. Each entry below is **ready-to-paste** +issue text — copy the body into a new GitHub issue when you want +to actually surface it. + +The list also doubles as a vetted "if you want to help, here are +five concrete things to pick up" pointer in `CONTRIBUTING.md`. + +--- + +## Bridge + +### Add a property test for `SimpleScoringStrategy`'s monotonicity + +**Body:** + +> The scoring engine in `bridge/scoring.py` has invariants that +> aren't yet tested as properties: +> - More goals → strictly higher (or equal) `gold` reward +> - More tokens (up to the cap) → strictly higher (or equal) total +> - Adding a plan never decreases reward +> - `momentum_multiplier` is non-negative +> +> Add `bridge/tests/test_scoring_properties.py` that uses +> `hypothesis` (add to `requirements.txt`) to generate Sessions and +> assert these invariants. +> +> **Acceptance**: PR adds `hypothesis` to `requirements.txt`, ships +> ≥3 property tests, all pass under `pytest -q`. +> +> **Files**: `bridge/scoring.py` (no change needed), `bridge/tests/ +> test_scoring_properties.py` (new). + +--- + +### Make `bridge/audit_log.py` rotate on size + +**Body:** + +> The audit log is append-only with no rotation. For a long +> long-running campaign it'll grow indefinitely. +> +> Add a `max_bytes` constructor arg (default 10 MiB). When the file +> exceeds the cap, rename it to `audit.jsonl.1` and start fresh. +> Keep one prior generation; older ones are deleted. +> +> **Acceptance**: tests cover the rotation trigger (manually inflated +> file size), the prior-generation file lands at the expected path, +> and a third write doesn't accumulate beyond two files. +> +> **Files**: `bridge/audit_log.py`, `bridge/tests/test_audit_log.py` +> (additions only). + +--- + +## TUI + +### Add an "all sessions" sidebar panel for multi-session kingdoms + +**Body:** + +> `KingdomView` lists sessions inline today. Once a player has 5+ +> sessions the listing crowds out the rest of the kingdom summary. +> +> Add a `SessionListPanel` widget that lives to the right of the +> main view and shows just `name [state] momentum_multiplier` per +> session, scrollable. +> +> **Acceptance**: visible side-by-side with `KingdomView`, scrolls +> with the keyboard, shows live updates when `_on_state_change` +> fires. +> +> **Files**: `tui/app.py`, optional new `tui/widgets/session_list.py`. + +--- + +## Mod + +### Add a fourth and fifth city name to The Operators + +**Body:** + +> `mod/ClaudeKingdoms/jsons/Nations.json` lists ten city names for +> The Operators. The PRD's Long-Running Campaign needs more — add +> two new in-character medieval city names with a one-line +> description in `Tutorials.json`. +> +> **Acceptance**: 12 cities total in The Operators' `cities` array, +> JSON still parses (smoke test in `bridge/tests/test_mod_jsons.py`), +> tutorial text mentions one of the new names with thematic flavor. +> +> **Files**: `mod/ClaudeKingdoms/jsons/Nations.json`, +> `mod/ClaudeKingdoms/jsons/Tutorials.json`. + +--- + +## Docs + +### Audit `docs/audit-2026-05-08.md` for stale info and snapshot a fresh audit + +**Body:** + +> The audit file is a snapshot dated 2026-05-08. It's been amended +> with closure markers, but a fresh audit (against current `main`) +> would catch any items that have regressed or new gaps. +> +> Take 30 minutes, walk through every BR/BS/CL/GM/TU and acceptance +> criterion in the PRD, mark each as ✅/🟡/❌ against the current +> code, and ship `docs/audit-YYYY-MM-DD.md` with the current date. +> +> **Acceptance**: new audit doc exists, references the current +> `main` HEAD, every PRD requirement is covered. +> +> **Files**: `docs/audit-YYYY-MM-DD.md` (new). The original audit +> stays as the historical baseline. + +--- + +## How to use this file (maintainers) + +When you want to actually surface a `good first issue`: + +```bash +gh issue create --repo SolshineCode/ClaudeKingdoms \ + --title "" \ + --label "good first issue" \ + --body "<body from above>" +``` + +After opening, **link the GitHub issue back to this file** so the +list stays in sync. Strike-through entries here that are now open +on GitHub. + +## Acceptance criterion #13 status + +This file ships ready-to-open issue text. Whether the GitHub issue +is technically `good first issue`-labeled at any moment is a small +maintainer step away. The architectural prerequisite — having a +solid candidate list — is met. diff --git a/tui/bridge_view.py b/tui/bridge_view.py new file mode 100644 index 0000000..727c02b --- /dev/null +++ b/tui/bridge_view.py @@ -0,0 +1,164 @@ +"""TUI-side schema-only adapter (PRD §TU-9). + +PRD §TU-9: 'TUI and Bridge communicate exclusively via the versioned +Bridge–TUI JSON schema contract defined above. TUI contributors must +not consume any bridge state outside this schema.' + +This module is the choke point. The TUI imports BridgeView and reads +its public fields; it does not import SessionState or Session directly +for rendering. Anything not in the PRD-mandated schema raises an +explicit AttributeError so contributors notice immediately. + +Construction: + bv = BridgeView.from_state(state) # in-process convenience + bv = BridgeView.from_payload(payload_dict) # the strict schema path +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Dict, List, Optional + + +# Mirrors PRD §"Bridge–TUI Interface Contract" line 231 minimum required +# per-session fields. Anything not in this set is bridge-internal and +# the TUI is forbidden from reading it. +SCHEMA_PER_SESSION_FIELDS = ( + "schema_version", + "session_id", + "state", + "momentum_streak", + "momentum_multiplier", + "base_value_this_turn", + "token_bonus", + "plan_bonus", + "goal_bonus", + "net_payout", + "last_event", + "last_event_ts", + "is_manual_injection", +) + +SCHEMA_TOP_LEVEL_FIELDS = ( + "schema_version", + "version", + "campaign_id", + "current_turn", + "kingdom_name", + "save_time", + "sessions", + "per_city_rewards", + "per_city_status", + "per_city_events", +) + + +@dataclass(frozen=True) +class SessionView: + """Schema-only view of a single session (PRD §TU-9 binding contract).""" + session_id: str + state: str # PRD-mandated; same value as legacy 'status' + momentum_streak: int + momentum_multiplier: int + base_value_this_turn: int + token_bonus: int + plan_bonus: int + goal_bonus: int + net_payout: int + last_event: Optional[str] + last_event_ts: Optional[str] + is_manual_injection: bool + schema_version: str + # Convenience fields outside the strict schema but useful for + # display; kept distinct so they're not confused with the contract. + name: str = "" + city_id: str = "" + + @classmethod + def from_dict(cls, d: Dict[str, Any]) -> "SessionView": + return cls( + session_id=d.get("session_id", ""), + state=d.get("state") or d.get("status", ""), # accept legacy alias + momentum_streak=int(d.get("momentum_streak") or d.get("momentum") or 0), + momentum_multiplier=int(d.get("momentum_multiplier", 0)), + base_value_this_turn=int(d.get("base_value_this_turn", 0)), + token_bonus=int(d.get("token_bonus", 0)), + plan_bonus=int(d.get("plan_bonus", 0)), + goal_bonus=int(d.get("goal_bonus", 0)), + net_payout=int(d.get("net_payout", 0)), + last_event=d.get("last_event"), + last_event_ts=d.get("last_event_ts"), + is_manual_injection=bool(d.get("is_manual_injection", False)), + schema_version=d.get("schema_version", ""), + name=d.get("name", ""), + city_id=d.get("city_id", ""), + ) + + +@dataclass(frozen=True) +class BridgeView: + """Schema-only view of the kingdom (PRD §TU-9 binding contract). + + Construct via from_payload (strict schema path) or from_state + (in-process convenience that calls state.to_dict() and adds the + enrichments SaveExchange would). + """ + schema_version: str + current_turn: int + kingdom_name: str + save_time: str + sessions: List[SessionView] + per_city_rewards: Dict[str, Dict[str, int]] + per_city_status: Dict[str, str] + per_city_events: Dict[str, List[Dict[str, Any]]] + + @classmethod + def from_payload(cls, payload: Dict[str, Any]) -> "BridgeView": + """Construct from a SaveExchange payload dict (strict schema path).""" + sessions = [SessionView.from_dict(s) for s in payload.get("sessions", [])] + return cls( + schema_version=payload.get("schema_version") or payload.get("version", ""), + current_turn=int(payload.get("current_turn", 0)), + kingdom_name=payload.get("kingdom_name", ""), + save_time=payload.get("save_time", ""), + sessions=sessions, + per_city_rewards=dict(payload.get("per_city_rewards") or {}), + per_city_status=dict(payload.get("per_city_status") or {}), + per_city_events=dict(payload.get("per_city_events") or {}), + ) + + @classmethod + def from_state(cls, state, scoring=None) -> "BridgeView": + """In-process convenience: serialize SessionState through the same + path SaveExchange uses, so the resulting BridgeView is byte-equivalent + to one read from disk. + """ + # Local imports to keep the TUI-side module independent of bridge + # internals at import time. + from bridge.scoring import ScoringEngine, SimpleScoringStrategy + strategy = SimpleScoringStrategy() + engine = ScoringEngine(strategy) if scoring is None else scoring + + payload = state.to_dict() + active_sessions = state.get_active_sessions() + if active_sessions: + payload["per_city_rewards"] = engine.calculate_per_city_rewards(active_sessions) + else: + payload["per_city_rewards"] = {} + payload["per_city_status"] = engine.calculate_per_city_status(state.sessions) + payload["per_city_events"] = getattr(state, "_pending_per_city_events", {}) or {} + + for session_dict, session in zip(payload.get("sessions", []), state.sessions): + session_dict.update(strategy.calculate_session_breakdown(session)) + session_dict["schema_version"] = "1.0.0" + + payload["schema_version"] = "1.0.0" + return cls.from_payload(payload) + + def total_rewards(self) -> Dict[str, int]: + """Sum of per-city rewards across all cities.""" + total = {"gold": 0, "science": 0, "culture": 0, "production": 0} + for bucket in self.per_city_rewards.values(): + for k, v in bucket.items(): + total[k] = total.get(k, 0) + v + return total diff --git a/tui/tests/test_bridge_view.py b/tui/tests/test_bridge_view.py new file mode 100644 index 0000000..8655d8d --- /dev/null +++ b/tui/tests/test_bridge_view.py @@ -0,0 +1,174 @@ +"""Tests for the TU-9 BridgeView adapter (TUI's schema-only entrypoint). + +PRD §TU-9: 'TUI and Bridge communicate exclusively via the versioned +Bridge–TUI JSON schema contract.' +""" + +import json +from datetime import datetime + +import pytest + +from bridge.save_exchange import SaveExchange +from bridge.scoring import ScoringEngine, SimpleScoringStrategy +from bridge.session_state import Session, SessionState, SessionStatus +from tui.bridge_view import ( + BridgeView, + SCHEMA_PER_SESSION_FIELDS, + SCHEMA_TOP_LEVEL_FIELDS, + SessionView, +) + + +def _seeded_state() -> SessionState: + state = SessionState(kingdom_name="Camelot", current_turn=4) + s = state.add_session("Royal Court") + s.session_id = "sess-1" + s.city_id = "camelot" + s.status = SessionStatus.ACTIVE + s.momentum = 12 + s.tokens = 5 + s.goals_completed = 1 + s.plans = ["p1"] + return state + + +# ──────────────────────────────────────────────────────────────────────── +# from_state — in-process construction +# ──────────────────────────────────────────────────────────────────────── + +def test_from_state_populates_top_level_fields(): + state = _seeded_state() + bv = BridgeView.from_state(state) + assert bv.kingdom_name == "Camelot" + assert bv.current_turn == 4 + assert bv.schema_version == "1.0.0" + assert len(bv.sessions) == 1 + + +def test_from_state_includes_breakdown_and_status(): + state = _seeded_state() + bv = BridgeView.from_state(state) + s = bv.sessions[0] + assert s.session_id == "sess-1" + assert s.state == "active" + assert s.momentum_streak == 12 + assert s.momentum_multiplier == 1 # 12 // 10 + assert s.base_value_this_turn > 0 + assert s.goal_bonus > 0 + assert s.net_payout > 0 + assert bv.per_city_status == {"camelot": "active"} + + +def test_from_state_includes_per_city_events_when_present(): + state = _seeded_state() + state._pending_per_city_events = {"camelot": [{"kind": "plan", + "session_id": "sess-1", + "session_name": "Royal Court"}]} + bv = BridgeView.from_state(state) + assert bv.per_city_events == {"camelot": [{"kind": "plan", + "session_id": "sess-1", + "session_name": "Royal Court"}]} + + +# ──────────────────────────────────────────────────────────────────────── +# from_payload — strict schema path +# ──────────────────────────────────────────────────────────────────────── + +def test_from_payload_round_trips_through_save_exchange(tmp_path): + """Read what SaveExchange wrote — strict TU-9 path.""" + state = _seeded_state() + ex = SaveExchange(tmp_path / "saves") + ex.write_payload(state) + on_disk = json.loads(ex.path.read_text(encoding="utf-8")) + bv = BridgeView.from_payload(on_disk) + assert bv.kingdom_name == "Camelot" + assert bv.schema_version == "1.0.0" + assert len(bv.sessions) == 1 + assert bv.sessions[0].session_id == "sess-1" + + +def test_from_payload_falls_back_to_legacy_aliases(): + """An older payload using 'status' / 'momentum' should still parse.""" + payload = { + "version": "1.0.0", + "campaign_id": "", + "current_turn": 0, + "kingdom_name": "Old", + "save_time": "2026-05-08T00:00:00", + "sessions": [{ + "session_id": "x", + "status": "active", # legacy alias + "momentum": 5, # legacy alias + }], + } + bv = BridgeView.from_payload(payload) + assert bv.sessions[0].state == "active" + assert bv.sessions[0].momentum_streak == 5 + + +def test_from_payload_handles_empty_sessions(): + payload = {"schema_version": "1.0.0", "current_turn": 0, + "kingdom_name": "Empty", "save_time": "", "sessions": []} + bv = BridgeView.from_payload(payload) + assert bv.sessions == [] + assert bv.kingdom_name == "Empty" + + +def test_from_payload_default_values_for_missing_per_city(): + payload = {"sessions": []} + bv = BridgeView.from_payload(payload) + assert bv.per_city_rewards == {} + assert bv.per_city_status == {} + assert bv.per_city_events == {} + + +# ──────────────────────────────────────────────────────────────────────── +# Aggregation helpers +# ──────────────────────────────────────────────────────────────────────── + +def test_total_rewards_sums_across_cities(): + payload = { + "schema_version": "1.0.0", "current_turn": 0, + "kingdom_name": "X", "save_time": "", "sessions": [], + "per_city_rewards": { + "a": {"gold": 10, "science": 5, "culture": 0, "production": 2}, + "b": {"gold": 1, "science": 1, "culture": 1, "production": 1}, + }, + } + bv = BridgeView.from_payload(payload) + assert bv.total_rewards() == {"gold": 11, "science": 6, "culture": 1, "production": 3} + + +def test_total_rewards_for_empty_kingdom(): + payload = {"sessions": []} + bv = BridgeView.from_payload(payload) + assert bv.total_rewards() == {"gold": 0, "science": 0, "culture": 0, "production": 0} + + +# ──────────────────────────────────────────────────────────────────────── +# Schema-discipline guarantees +# ──────────────────────────────────────────────────────────────────────── + +def test_schema_field_constants_match_prd_minimum(): + """Sanity: the constants stay aligned with PRD line 231 wording.""" + expected_per_session = { + "schema_version", "session_id", "state", "momentum_streak", + "momentum_multiplier", "base_value_this_turn", "token_bonus", + "plan_bonus", "goal_bonus", "net_payout", "last_event", + "last_event_ts", "is_manual_injection", + } + assert set(SCHEMA_PER_SESSION_FIELDS) == expected_per_session + + +def test_session_view_is_immutable(): + """Frozen dataclass: contributors can't mutate the schema-side state.""" + sv = SessionView.from_dict({"session_id": "x", "state": "active"}) + with pytest.raises(Exception): + sv.session_id = "different" # type: ignore[misc] + + +def test_bridge_view_is_immutable(): + bv = BridgeView.from_payload({"sessions": []}) + with pytest.raises(Exception): + bv.kingdom_name = "different" # type: ignore[misc]