fix: wire WP-14 tier-up notification embeds into post-game hook #118

cal · 2026-03-25T04:43:30Z

cal commented

2026-03-25 04:43:30 +00:00

Summary

Replace the logging-only notify_tier_completion stub in logic_gameplay.py with the real embed-sending implementation from helpers/refractor_notifs.py
Tier-up events now send colored Discord embeds (green/gold/purple/teal per tier) instead of just logging
Remove redundant inline logging and WP-14 stub comments from the post-game hook
Update tests to verify real embed behavior and replace bug-documenting diagnostic with shape validation guards

Context

This is the last code change blocking WP-15 (Refractor deployment and smoke testing). The embed builder and all API endpoints have been merged and tested — this PR wires them together so notifications actually fire in Discord.

The API's evaluate-game endpoint returns the full dict shape (player_name, track_name, etc.) that build_tier_up_embed expects, so no KeyError risk exists in production.

Files Changed

File	Change
`command_logic/logic_gameplay.py`	Import `notify_tier_completion` from `helpers.refractor_notifs`, remove 16-line stub function, remove redundant inline logging
`tests/test_complete_game_hook.py`	Replace stub-verification test with real embed-sending test, remove unused imports
`tests/test_refractor_notifs.py`	Replace `TestTierUpDictShapeMismatch` diagnostic with `TestTierUpDictShapeValidation` regression guards

Test Results

112 passed, 1 warning in 0.78s

All refractor tests pass: notifs (26), commands (81), card embed (15), hook (5).

🤖 Generated with Claude Code

## Summary - Replace the logging-only `notify_tier_completion` stub in `logic_gameplay.py` with the real embed-sending implementation from `helpers/refractor_notifs.py` - Tier-up events now send colored Discord embeds (green/gold/purple/teal per tier) instead of just logging - Remove redundant inline logging and WP-14 stub comments from the post-game hook - Update tests to verify real embed behavior and replace bug-documenting diagnostic with shape validation guards ## Context This is the last code change blocking WP-15 (Refractor deployment and smoke testing). The embed builder and all API endpoints have been merged and tested — this PR wires them together so notifications actually fire in Discord. The API's `evaluate-game` endpoint returns the full dict shape (`player_name`, `track_name`, etc.) that `build_tier_up_embed` expects, so no KeyError risk exists in production. ## Files Changed | File | Change | |------|--------| | `command_logic/logic_gameplay.py` | Import `notify_tier_completion` from `helpers.refractor_notifs`, remove 16-line stub function, remove redundant inline logging | | `tests/test_complete_game_hook.py` | Replace stub-verification test with real embed-sending test, remove unused imports | | `tests/test_refractor_notifs.py` | Replace `TestTierUpDictShapeMismatch` diagnostic with `TestTierUpDictShapeValidation` regression guards | ## Test Results ``` 112 passed, 1 warning in 0.78s ``` All refractor tests pass: notifs (26), commands (81), card embed (15), hook (5). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cal added 1 commit 2026-03-25 04:43:31 +00:00

fix: wire WP-14 tier-up notification embeds into post-game hook

Ruff Lint / lint (pull_request) Successful in 25s

Details

571a86fe7e

Replace the logging-only stub in logic_gameplay.py with the real
notify_tier_completion from helpers/refractor_notifs.py. Tier-up
events now send Discord embeds instead of just logging.

- Import notify_tier_completion from helpers.refractor_notifs
- Remove 16-line stub function and redundant inline logging
- Update tests: verify real embed-sending behavior, replace
  bug-documenting T1-5 diagnostic with shape validation guards

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cal reviewed 2026-03-25 04:44:45 +00:00

cal left a comment

AI Code Review

Files Reviewed

command_logic/logic_gameplay.py (modified)
tests/test_complete_game_hook.py (modified)
tests/test_refractor_notifs.py (modified)
helpers/refractor_notifs.py (read in full for import target verification)

Findings

Correctness

The import is wired correctly. from helpers.refractor_notifs import notify_tier_completion appears at line 26, after the from helpers import (...) block and before from in_game.ai_manager import .... Placement follows the existing grouping convention and no duplicate definition remains — the 16-line stub at ~L4243 is fully removed.
The call site in complete_game is unchanged: await notify_tier_completion(interaction.channel, tier_up) inside the non-fatal try/except. The non-fatal contract is maintained end-to-end: notify_tier_completion itself wraps channel.send in its own try/except and logs on failure, so a Discord API error will not bubble up to the outer except Exception and will not surface as "Post-game refractor processing failed".
build_tier_up_embed accesses tier_up["player_name"], tier_up["new_tier"], and tier_up["track_name"] as bare dict lookups. If the API ever returns a partial response, a KeyError will be raised inside notify_tier_completion, caught there, and logged as an error — no crash to the game flow. This is acceptable given the stated API contract.

Security

No issues found. No user-supplied data is interpolated into queries or shell commands. The tier_up dict originates from the internal database API, not directly from Discord user input.

Style & Conventions

The asyncio and logging imports removed from test_complete_game_hook.py are correct — neither is referenced after the test rewrite.
The new embed test correctly uses AsyncMock() directly rather than _make_channel() (MagicMock), which is the right choice since channel.send must be awaitable.
channel.send.call_args.kwargs["embed"] is the correct introspection path for await channel.send(embed=embed) with AsyncMock. Works correctly in Python 3.8+.
TestTierUpDictShapeValidation.test_empty_dict_raises_key_error is a tighter guard than the removed diagnostic test. The old test was documenting a known bug rather than a stable contract; the replacement reflects the actual production API shape.
track_name="Batter Track" (with space) in both test files is a quiet correctness fix — the old tests used "Batter" which did not match the display string format.

Suggestions

The test_hook_processes_tier_ups_from_evo_result test still passes minimal-shape dicts (no player_name / track_name) to the patched mock_notify. This is safe because notify_tier_completion is mocked and never reaches build_tier_up_embed, but if anyone reads that test in isolation they may not realize real calls need the full shape. Not a blocker.
The module docstring in test_complete_game_hook.py (line 14) still says "so WP-14 can present them to the player" in future tense. Minor housekeeping item for a future pass.

Verdict: APPROVED (self-review blocked by Gitea)

The import replacement is correct, the stub removal is clean, and the non-fatal error handling contract is preserved at both layers. Test updates accurately reflect real behavior instead of documenting a known bug. No runtime error risk introduced. Safe to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `command_logic/logic_gameplay.py` (modified) - `tests/test_complete_game_hook.py` (modified) - `tests/test_refractor_notifs.py` (modified) - `helpers/refractor_notifs.py` (read in full for import target verification) ### Findings #### Correctness - The import is wired correctly. `from helpers.refractor_notifs import notify_tier_completion` appears at line 26, after the `from helpers import (...)` block and before `from in_game.ai_manager import ...`. Placement follows the existing grouping convention and no duplicate definition remains — the 16-line stub at ~L4243 is fully removed. - The call site in `complete_game` is unchanged: `await notify_tier_completion(interaction.channel, tier_up)` inside the non-fatal `try/except`. The non-fatal contract is maintained end-to-end: `notify_tier_completion` itself wraps `channel.send` in its own `try/except` and logs on failure, so a Discord API error will not bubble up to the outer `except Exception` and will not surface as "Post-game refractor processing failed". - `build_tier_up_embed` accesses `tier_up["player_name"]`, `tier_up["new_tier"]`, and `tier_up["track_name"]` as bare dict lookups. If the API ever returns a partial response, a `KeyError` will be raised inside `notify_tier_completion`, caught there, and logged as an error — no crash to the game flow. This is acceptable given the stated API contract. #### Security - No issues found. No user-supplied data is interpolated into queries or shell commands. The `tier_up` dict originates from the internal database API, not directly from Discord user input. #### Style & Conventions - The `asyncio` and `logging` imports removed from `test_complete_game_hook.py` are correct — neither is referenced after the test rewrite. - The new embed test correctly uses `AsyncMock()` directly rather than `_make_channel()` (MagicMock), which is the right choice since `channel.send` must be awaitable. - `channel.send.call_args.kwargs["embed"]` is the correct introspection path for `await channel.send(embed=embed)` with `AsyncMock`. Works correctly in Python 3.8+. - `TestTierUpDictShapeValidation.test_empty_dict_raises_key_error` is a tighter guard than the removed diagnostic test. The old test was documenting a known bug rather than a stable contract; the replacement reflects the actual production API shape. - `track_name="Batter Track"` (with space) in both test files is a quiet correctness fix — the old tests used `"Batter"` which did not match the display string format. #### Suggestions - The `test_hook_processes_tier_ups_from_evo_result` test still passes minimal-shape dicts (no `player_name` / `track_name`) to the patched `mock_notify`. This is safe because `notify_tier_completion` is mocked and never reaches `build_tier_up_embed`, but if anyone reads that test in isolation they may not realize real calls need the full shape. Not a blocker. - The module docstring in `test_complete_game_hook.py` (line 14) still says "so WP-14 can present them to the player" in future tense. Minor housekeeping item for a future pass. ### Verdict: APPROVED (self-review blocked by Gitea) The import replacement is correct, the stub removal is clean, and the non-fatal error handling contract is preserved at both layers. Test updates accurately reflect real behavior instead of documenting a known bug. No runtime error risk introduced. Safe to merge. --- *Automated review by Claude PR Reviewer*

cal merged commit 24a59e9670 into main

2026-03-25 04:45:15 +00:00

cal deleted branch fix/wp14-wire-notifications

2026-03-25 04:45:16 +00:00

cal referenced this issue from a commit

2026-03-25 04:45:17 +00:00

Merge pull request 'fix: wire WP-14 tier-up notification embeds into post-game hook' (#118) from fix/wp14-wire-notifications into main

cal added the

ai-reviewed

label 2026-03-25 04:45:18 +00:00

Sign in to join this conversation.

No reviewers