test: add comprehensive refractor test cases (T1-5 through T3-5)

Implements all gap tests identified by PO agents across three existing
test files. No new files created — tests added to existing modules.

T1-5 (test_refractor_notifs): Expose WP-14 integration bug — minimal
stub dict {player_id, old_tier, new_tier} causes KeyError in
build_tier_up_embed because player_name/track_name use bare dict access.
Documents the bug contract so WP-14 implementers know what to fix.

T1-6 (test_refractor_commands): Divergence tripwire — imports TIER_NAMES
from both cogs.refractor and helpers.refractor_notifs and asserts deep
equality. Will fail the moment the two copies fall out of sync.

T1-7 (test_card_embed_refractor): TIER_BADGES format contract — asserts
that wrapping helpers.main badge values in brackets produces cogs.refractor
badge values (e.g. "BC" -> "[BC]") for all tiers.

T2-7 (test_refractor_notifs): notify_tier_completion with None channel
must not raise — the try/except absorbs AttributeError from None.send().

T2-8 (test_refractor_commands): All-T4 apply_close_filter returns empty
list. Documents intended behaviour for tier=4 + progress="close" combo.

T3-2 (test_refractor_commands): Malformed API response handling —
format_refractor_entry must use fallbacks ("Unknown", 0) for missing keys.

T3-3 (test_refractor_commands): Progress bar boundary precision — 1/100,
99/100, 0/100, and negative current values.

T3-4 (test_refractor_commands): RP formula label — card_type="rp" shows
"IP+K" (previously only "sp" was tested).

T3-5 (test_refractor_commands): Unknown card_type falls back to raw string
as the formula label without crashing.

112 tests pass (23 new, 89 pre-existing).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

tests/test_card_embed_refractor.py

@@ -246,6 +246,85 @@ class TestEmbedColorUnchanged:
         assert embeds[0].color == discord.Color(int(rarity_color, 16))
 
 
+# ---------------------------------------------------------------------------
+# T1-7: TIER_BADGES format consistency check across modules
+# ---------------------------------------------------------------------------
+
+
+class TestTierBadgesFormatConsistency:
+    """
+    T1-7: Assert that TIER_BADGES in cogs.refractor (format: "[BC]") and
+    helpers.main (format: "BC") are consistent — wrapping the helpers.main
+    value in brackets must produce the cogs.refractor value.
+
+    Why: The two modules intentionally use different formats for different
+    rendering contexts:
+      - helpers.main uses bare strings ("BC") because get_card_embeds
+        wraps them in brackets when building the embed title.
+      - cogs.refractor uses bracket strings ("[BC]") because
+        format_refractor_entry inlines them directly into the display string.
+
+    If either definition is updated without updating the other, embed titles
+    and /refractor status output will display inconsistent badges.  This test
+    acts as an explicit contract check so any future change to either dict
+    is immediately surfaced here.
+    """
+
+    def test_cogs_badge_equals_bracketed_helpers_badge_for_all_tiers(self):
+        """
+        For every tier in cogs.refractor TIER_BADGES, wrapping the
+        helpers.main TIER_BADGES value in square brackets must produce
+        the cogs.refractor value.
+
+        i.e., f"[{helpers_badge}]" == cog_badge for all tiers.
+        """
+        from cogs.refractor import TIER_BADGES as cog_badges
+        from helpers.main import TIER_BADGES as helpers_badges
+
+        assert set(cog_badges.keys()) == set(helpers_badges.keys()), (
+            "TIER_BADGES key sets differ between cogs.refractor and helpers.main. "
+            f"cogs keys: {set(cog_badges.keys())}, helpers keys: {set(helpers_badges.keys())}"
+        )
+
+        for tier, cog_badge in cog_badges.items():
+            helpers_badge = helpers_badges[tier]
+            expected = f"[{helpers_badge}]"
+            assert cog_badge == expected, (
+                f"Tier {tier} badge mismatch: "
+                f"cogs.refractor={cog_badge!r}, "
+                f"helpers.main={helpers_badge!r} "
+                f"(expected cog badge to equal '[{helpers_badge}]')"
+            )
+
+    def test_t1_badge_relationship(self):
+        """T1: helpers.main 'BC' wrapped in brackets equals cogs.refractor '[BC]'."""
+        from cogs.refractor import TIER_BADGES as cog_badges
+        from helpers.main import TIER_BADGES as helpers_badges
+
+        assert f"[{helpers_badges[1]}]" == cog_badges[1]
+
+    def test_t2_badge_relationship(self):
+        """T2: helpers.main 'R' wrapped in brackets equals cogs.refractor '[R]'."""
+        from cogs.refractor import TIER_BADGES as cog_badges
+        from helpers.main import TIER_BADGES as helpers_badges
+
+        assert f"[{helpers_badges[2]}]" == cog_badges[2]
+
+    def test_t3_badge_relationship(self):
+        """T3: helpers.main 'GR' wrapped in brackets equals cogs.refractor '[GR]'."""
+        from cogs.refractor import TIER_BADGES as cog_badges
+        from helpers.main import TIER_BADGES as helpers_badges
+
+        assert f"[{helpers_badges[3]}]" == cog_badges[3]
+
+    def test_t4_badge_relationship(self):
+        """T4: helpers.main 'SF' wrapped in brackets equals cogs.refractor '[SF]'."""
+        from cogs.refractor import TIER_BADGES as cog_badges
+        from helpers.main import TIER_BADGES as helpers_badges
+
+        assert f"[{helpers_badges[4]}]" == cog_badges[4]
+
+
 # ---------------------------------------------------------------------------
 # Helper: call get_card_embeds and return embed list
 # ---------------------------------------------------------------------------

tests/test_refractor_commands.py

@@ -186,8 +186,6 @@ class TestFormatRefractorEntry:
         assert len(lines) == 2
 
 
-
-
 # ---------------------------------------------------------------------------
 # TIER_BADGES
 # ---------------------------------------------------------------------------
@@ -257,6 +255,7 @@ class TestTierBadges:
         name_pos = first_line.find("Mike Trout")
         assert badge_pos < name_pos
 
+
 # ---------------------------------------------------------------------------
 # apply_close_filter
 # ---------------------------------------------------------------------------
@@ -421,6 +420,342 @@ def mock_interaction():
     return interaction
 
 
+# ---------------------------------------------------------------------------
+# T1-6: TIER_NAMES duplication divergence check
+# ---------------------------------------------------------------------------
+
+
+class TestTierNamesDivergenceCheck:
+    """
+    T1-6: Assert that TIER_NAMES in cogs.refractor and helpers.refractor_notifs
+    are identical (same keys, same values).
+
+    Why: TIER_NAMES is duplicated in two modules.  If one is updated and the
+    other is not (e.g. a tier is renamed or a new tier is added), tier labels
+    in the /refractor status embed and the tier-up notification embed will
+    diverge silently.  This test acts as a divergence tripwire — it will fail
+    the moment the two copies fall out of sync, forcing an explicit fix.
+    """
+
+    def test_tier_names_are_identical_across_modules(self):
+        """
+        Import TIER_NAMES from both modules and assert deep equality.
+
+        The test imports the name at call-time rather than at module level to
+        ensure it always reads the current definition and is not affected by
+        module-level caching or monkeypatching in other tests.
+        """
+        from cogs.refractor import TIER_NAMES as cog_tier_names
+        from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names
+
+        assert cog_tier_names == notifs_tier_names, (
+            "TIER_NAMES differs between cogs.refractor and helpers.refractor_notifs. "
+            "Both copies must be kept in sync. "
+            f"cogs.refractor: {cog_tier_names!r}  "
+            f"helpers.refractor_notifs: {notifs_tier_names!r}"
+        )
+
+    def test_tier_names_have_same_keys(self):
+        """Keys (tier numbers) must be identical in both modules."""
+        from cogs.refractor import TIER_NAMES as cog_tier_names
+        from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names
+
+        assert set(cog_tier_names.keys()) == set(notifs_tier_names.keys()), (
+            "TIER_NAMES key sets differ between modules."
+        )
+
+    def test_tier_names_have_same_values(self):
+        """Display strings (values) must be identical for every shared key."""
+        from cogs.refractor import TIER_NAMES as cog_tier_names
+        from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names
+
+        for tier, name in cog_tier_names.items():
+            assert notifs_tier_names.get(tier) == name, (
+                f"Tier {tier} name mismatch: "
+                f"cogs.refractor={name!r}, "
+                f"helpers.refractor_notifs={notifs_tier_names.get(tier)!r}"
+            )
+
+
+# ---------------------------------------------------------------------------
+# T2-8: Filter combination — tier=4 + progress="close" yields empty result
+# ---------------------------------------------------------------------------
+
+
+class TestApplyCloseFilterWithAllT4Cards:
+    """
+    T2-8: When all cards in the list are T4 (fully evolved), apply_close_filter
+    must return an empty list.
+
+    Why: T4 cards have no next tier to advance to, so they have no threshold.
+    The close filter explicitly excludes fully evolved cards (tier >= 4 or
+    next_threshold is None).  If a user passes both tier=4 and progress="close"
+    to /refractor status, the combined result should be empty — the command
+    already handles this by showing "No cards are currently close to a tier
+    advancement."  This test documents and protects that behaviour.
+    """
+
+    def test_all_t4_cards_returns_empty(self):
+        """
+        A list of T4-only card states should produce an empty result from
+        apply_close_filter, because T4 cards are fully evolved and have no
+        next threshold to be "close" to.
+
+        This is the intended behaviour when tier=4 and progress="close" are
+        combined: there are no qualifying cards, and the command should show
+        the "no cards close to advancement" message rather than an empty embed.
+        """
+        t4_cards = [
+            {"current_tier": 4, "formula_value": 300, "next_threshold": None},
+            {"current_tier": 4, "formula_value": 500, "next_threshold": None},
+            {"current_tier": 4, "formula_value": 275, "next_threshold": None},
+        ]
+        result = apply_close_filter(t4_cards)
+        assert result == [], (
+            "apply_close_filter must return [] for fully evolved T4 cards — "
+            "they have no next threshold and cannot be 'close' to advancement."
+        )
+
+    def test_t4_cards_excluded_even_with_high_formula_value(self):
+        """
+        T4 cards are excluded regardless of their formula_value, since the
+        filter is based on tier (>= 4) and threshold (None), not raw values.
+        """
+        t4_high_value = {
+            "current_tier": 4,
+            "formula_value": 9999,
+            "next_threshold": None,
+        }
+        assert apply_close_filter([t4_high_value]) == []
+
+
+# ---------------------------------------------------------------------------
+# T3-2: Malformed API response handling in format_refractor_entry
+# ---------------------------------------------------------------------------
+
+
+class TestFormatRefractorEntryMalformedInput:
+    """
+    T3-2: format_refractor_entry should not crash when given a card state dict
+    that is missing expected keys.
+
+    Why: API responses can be incomplete due to migration states, partially
+    populated records, or future schema changes.  format_refractor_entry uses
+    .get() with fallbacks for all keys, so missing fields should gracefully
+    degrade to sensible defaults ("Unknown" for name, 0 for values) rather than
+    raising a KeyError or TypeError.
+    """
+
+    def test_missing_player_name_uses_unknown(self):
+        """
+        When player_name is absent, the output should contain "Unknown" rather
+        than crashing with a KeyError.
+        """
+        state = {
+            "card_type": "batter",
+            "current_tier": 1,
+            "formula_value": 100,
+            "next_threshold": 150,
+        }
+        result = format_refractor_entry(state)
+        assert "Unknown" in result
+
+    def test_missing_formula_value_uses_zero(self):
+        """
+        When formula_value is absent, the progress calculation should use 0
+        without raising a TypeError.
+        """
+        state = {
+            "player_name": "Test Player",
+            "card_type": "batter",
+            "current_tier": 1,
+            "next_threshold": 150,
+        }
+        result = format_refractor_entry(state)
+        assert "0/150" in result
+
+    def test_completely_empty_dict_does_not_crash(self):
+        """
+        An entirely empty dict should produce a valid (if sparse) string using
+        all fallback values, not raise any exception.
+        """
+        result = format_refractor_entry({})
+        # Should not raise; output should be a string with two lines
+        assert isinstance(result, str)
+        lines = result.split("\n")
+        assert len(lines) == 2
+
+    def test_missing_card_type_uses_raw_fallback(self):
+        """
+        When card_type is absent, the code defaults to 'batter' internally
+        (via .get("card_type", "batter")), so "PA+TB×2" should appear as the
+        formula label.
+        """
+        state = {
+            "player_name": "Test Player",
+            "current_tier": 1,
+            "formula_value": 50,
+            "next_threshold": 100,
+        }
+        result = format_refractor_entry(state)
+        assert "PA+TB×2" in result
+
+
+# ---------------------------------------------------------------------------
+# T3-3: Progress bar boundary precision
+# ---------------------------------------------------------------------------
+
+
+class TestRenderProgressBarBoundaryPrecision:
+    """
+    T3-3: Verify the progress bar behaves correctly at edge values — near zero,
+    near full, exactly at extremes, and for negative input.
+
+    Why: Off-by-one errors in rounding or integer truncation can make a nearly-
+    full bar look full (or vice versa), confusing users about how close their
+    card is to a tier advancement.  Defensive handling of negative values ensures
+    no bar is rendered longer than its declared width.
+    """
+
+    def test_one_of_hundred_shows_mostly_empty(self):
+        """
+        1/100 = 1% — should produce a bar with 0 or 1 filled segment and the
+        rest empty.  The bar must not appear more than minimally filled.
+        """
+        bar = render_progress_bar(1, 100)
+        # Interior is 10 chars: count '=' vs '-'
+        interior = bar[1:-1]  # strip '[' and ']'
+        filled_count = interior.count("=")
+        assert filled_count <= 1, (
+            f"1/100 should show 0 or 1 filled segment, got {filled_count}: {bar!r}"
+        )
+
+    def test_ninety_nine_of_hundred_is_nearly_full(self):
+        """
+        99/100 = 99% — should produce a bar with 9 or 10 filled segments.
+        The bar must NOT be completely empty or show fewer than 9 filled.
+        """
+        bar = render_progress_bar(99, 100)
+        interior = bar[1:-1]
+        filled_count = interior.count("=")
+        assert filled_count >= 9, (
+            f"99/100 should show 9 or 10 filled segments, got {filled_count}: {bar!r}"
+        )
+        # But it must not overflow the bar width
+        assert len(interior) == 10
+
+    def test_zero_of_hundred_is_completely_empty(self):
+        """0/100 = all dashes — re-verify the all-empty baseline."""
+        assert render_progress_bar(0, 100) == "[----------]"
+
+    def test_negative_current_does_not_overflow_bar(self):
+        """
+        A negative formula_value (data anomaly) must not produce a bar with
+        more filled segments than the width.  The min(..., 1.0) clamp in
+        render_progress_bar should handle this, but this test guards against
+        a future refactor removing the clamp.
+        """
+        bar = render_progress_bar(-5, 100)
+        interior = bar[1:-1]
+        # No filled segments should exist for a negative value
+        filled_count = interior.count("=")
+        assert filled_count == 0, (
+            f"Negative current should produce 0 filled segments, got {filled_count}: {bar!r}"
+        )
+        # Bar width must be exactly 10
+        assert len(interior) == 10
+
+
+# ---------------------------------------------------------------------------
+# T3-4: RP formula label
+# ---------------------------------------------------------------------------
+
+
+class TestRPFormulaLabel:
+    """
+    T3-4: Verify that relief pitchers (card_type="rp") show the "IP+K" formula
+    label in format_refractor_entry output.
+
+    Why: FORMULA_LABELS maps both "sp" and "rp" to "IP+K".  The existing test
+    suite only verifies "sp" (via the sp_state fixture).  Adding "rp" explicitly
+    prevents a future refactor from accidentally giving RPs a different label
+    or falling through to the raw card_type fallback.
+    """
+
+    def test_rp_formula_label_is_ip_plus_k(self):
+        """
+        A card with card_type="rp" must show "IP+K" as the formula label
+        in its progress line.
+        """
+        rp_state = {
+            "player_name": "Edwin Diaz",
+            "card_type": "rp",
+            "current_tier": 1,
+            "formula_value": 45,
+            "next_threshold": 60,
+        }
+        result = format_refractor_entry(rp_state)
+        assert "IP+K" in result, (
+            f"Relief pitcher card should show 'IP+K' formula label, got: {result!r}"
+        )
+
+
+# ---------------------------------------------------------------------------
+# T3-5: Unknown card_type fallback
+# ---------------------------------------------------------------------------
+
+
+class TestUnknownCardTypeFallback:
+    """
+    T3-5: format_refractor_entry should use the raw card_type string as the
+    formula label when the type is not in FORMULA_LABELS, rather than crashing.
+
+    Why: FORMULA_LABELS only covers "batter", "sp", and "rp".  If the API
+    introduces a new card type (e.g. "util" for utility players) before the
+    bot is updated, FORMULA_LABELS.get(card_type, card_type) will fall back to
+    the raw string.  This test ensures that fallback path produces readable
+    output rather than an error, and explicitly documents what to expect.
+    """
+
+    def test_unknown_card_type_uses_raw_string_as_label(self):
+        """
+        card_type="util" is not in FORMULA_LABELS.  The output should include
+        "util" as the formula label (the raw fallback) and must not raise.
+        """
+        util_state = {
+            "player_name": "Ben Zobrist",
+            "card_type": "util",
+            "current_tier": 2,
+            "formula_value": 80,
+            "next_threshold": 120,
+        }
+        result = format_refractor_entry(util_state)
+        assert "util" in result, (
+            f"Unknown card_type should appear verbatim as the formula label, got: {result!r}"
+        )
+
+    def test_unknown_card_type_does_not_crash(self):
+        """
+        Any unknown card_type must produce a valid two-line string without
+        raising an exception.
+        """
+        state = {
+            "player_name": "Test Player",
+            "card_type": "dh",
+            "current_tier": 1,
+            "formula_value": 30,
+            "next_threshold": 50,
+        }
+        result = format_refractor_entry(state)
+        assert isinstance(result, str)
+        assert len(result.split("\n")) == 2
+
+
+# ---------------------------------------------------------------------------
+# Slash command: empty roster / no-team scenarios
+# ---------------------------------------------------------------------------
+
+
 @pytest.mark.asyncio
 async def test_refractor_status_no_team(mock_bot, mock_interaction):
     """

tests/test_refractor_notifs.py

@@ -257,3 +257,92 @@ class TestNotifyTierCompletion:
         for event in tier_up_events:
             await notify_tier_completion(channel, event)
         channel.send.assert_not_called()
+
+
+# ---------------------------------------------------------------------------
+# T1-5: tier_up dict shape mismatch — WP-14 integration blocker
+# ---------------------------------------------------------------------------
+
+
+class TestTierUpDictShapeMismatch:
+    """
+    T1-5: Expose the latent integration bug where the post-game hook passes a
+    minimal tier_up dict (only player_id, old_tier, new_tier) but
+    build_tier_up_embed expects player_name, old_tier, new_tier, track_name,
+    and current_value.
+
+    Why this matters: the hook test (test_complete_game_hook.py) confirms the
+    plumbing forwards tier_up dicts from the API response to notify_tier_completion.
+    However, the real API response may omit player_name/track_name. If
+    build_tier_up_embed does a bare dict access (tier_up["player_name"]) without
+    a fallback, it will raise KeyError in production. This test documents the
+    current behaviour (crash vs. graceful degradation) so WP-14 implementers
+    know to either harden the embed builder or ensure the API always returns
+    the full shape.
+    """
+
+    def test_minimal_stub_shape_raises_key_error(self):
+        """
+        Calling build_tier_up_embed with only {player_id, old_tier, new_tier}
+        (the minimal shape used by the post-game hook stub) raises KeyError
+        because player_name and track_name are accessed via bare dict lookup.
+
+        This is the latent bug: the hook passes stub-shaped dicts but the embed
+        builder expects the full notification shape. WP-14 must ensure either
+        (a) the API returns the full shape or (b) build_tier_up_embed degrades
+        gracefully with .get() fallbacks.
+        """
+        minimal_stub = {
+            "player_id": 101,
+            "old_tier": 1,
+            "new_tier": 2,
+        }
+        # Document that this raises — it's the bug we're exposing, not a passing test.
+        with pytest.raises(KeyError):
+            build_tier_up_embed(minimal_stub)
+
+    def test_full_shape_does_not_raise(self):
+        """
+        Confirm that supplying the full expected shape (player_name, old_tier,
+        new_tier, track_name, current_value) does NOT raise, establishing the
+        correct contract for callers.
+        """
+        full_shape = make_tier_up(
+            player_name="Mike Trout",
+            old_tier=1,
+            new_tier=2,
+            track_name="Batter",
+            current_value=150,
+        )
+        # Must not raise
+        embed = build_tier_up_embed(full_shape)
+        assert embed is not None
+
+
+# ---------------------------------------------------------------------------
+# T2-7: notify_tier_completion with None channel
+# ---------------------------------------------------------------------------
+
+
+class TestNotifyTierCompletionNoneChannel:
+    """
+    T2-7: notify_tier_completion must not propagate exceptions when the channel
+    is None.
+
+    Why: the post-game hook may call notify_tier_completion before a valid
+    channel is resolved (e.g. in tests, or if the scoreboard channel lookup
+    fails). The try/except in notify_tier_completion should catch the
+    AttributeError from None.send() so game flow is never interrupted.
+    """
+
+    @pytest.mark.asyncio
+    async def test_none_channel_does_not_raise(self):
+        """
+        Passing None as the channel argument must not raise.
+
+        None.send() raises AttributeError; the try/except in
+        notify_tier_completion is expected to absorb it silently.
+        """
+        tier_up = make_tier_up(new_tier=2)
+        # Should not raise regardless of channel being None
+        await notify_tier_completion(None, tier_up)