-
Notifications
You must be signed in to change notification settings - Fork 0
feat(p2): TU-9 schema-only BridgeView adapter + good-first-issues catalog #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 "<title from above>" \ | ||
| --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. | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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 | ||||||||||
|
Comment on lines
+139
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In
Suggested change
|
||||||||||
|
|
||||||||||
| 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 | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file path for the new test file is split across two lines within the backticks. This will likely result in a broken path (containing a newline or space) when the issue body is copied or rendered. It is better to keep the full path on a single line to ensure it remains valid.