feat(p2): TU-9 schema-only BridgeView adapter + good-first-issues catalog#32
Conversation
…alog 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) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a 'good first issue' candidate list to the documentation and implements the BridgeView adapter to strictly enforce the Bridge-TUI interface contract. The reviewer identified a logic inconsistency in the from_state method regarding scoring strategies, a formatting issue in the documentation that splits a file path across lines, and suggested using more specific exception types in the immutability tests.
| strategy = SimpleScoringStrategy() | ||
| engine = ScoringEngine(strategy) if scoring is None else scoring |
There was a problem hiding this comment.
In from_state, if a custom scoring engine is provided, the code still uses a locally created SimpleScoringStrategy to calculate the session breakdown on line 152. This creates an inconsistency if the provided engine uses a different strategy. The method should instead use the strategy associated with the engine instance.
| strategy = SimpleScoringStrategy() | |
| engine = ScoringEngine(strategy) if scoring is None else scoring | |
| engine = scoring or ScoringEngine(SimpleScoringStrategy()) | |
| strategy = getattr(engine, "strategy", SimpleScoringStrategy()) |
| > **Files**: `bridge/scoring.py` (no change needed), `bridge/tests/ | ||
| > test_scoring_properties.py` (new). |
There was a problem hiding this comment.
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.
| > **Files**: `bridge/scoring.py` (no change needed), `bridge/tests/ | |
| > test_scoring_properties.py` (new). | |
| > **Files**: `bridge/scoring.py` (no change needed), | |
| > `bridge/tests/test_scoring_properties.py` (new). |
| with pytest.raises(Exception): | ||
| sv.session_id = "different" # type: ignore[misc] |
There was a problem hiding this comment.
Using a broad Exception in pytest.raises is discouraged as it might mask unexpected errors. For frozen dataclasses, the specific exception raised on mutation is dataclasses.FrozenInstanceError, which inherits from TypeError.
| with pytest.raises(Exception): | |
| sv.session_id = "different" # type: ignore[misc] | |
| with pytest.raises(TypeError): | |
| sv.session_id = "different" # type: ignore[misc] |
1 participant
Closes audit P2: TU-9 (TUI schema-only consumption choke point) and progress on acceptance #13 (good first issue candidates ready to surface). 12 new tests; 262 total. Refs PR #22, kanban t_26404be3.