From d1a6b57ccd2d36a78f728012f0910cb0e516dfde Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Fri, 20 Feb 2026 14:14:23 -0600 Subject: [PATCH] fix: scorebug stale data, win probability parsing, and read-failure tolerance (closes #39, #40) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #40: ScorecardTracker cached data in memory at startup — background task never saw newly published scorecards. Fixed by reloading from disk on every read. #39: Win percentage defaulted to 50% when unavailable, showing a misleading 50/50 bar. Now defaults to None with "unavailable" message in embed. Parsing handles decimal (0.75), percentage string, and empty values. Also fixed orientation bug where win% was always shown as home team's even when the sheet reports the away team as the leader. Additionally: live scorebug tracker now distinguishes between "all games confirmed final" and "sheet read failures" — transient Google Sheets errors no longer hide the live scores channel. Co-Authored-By: Claude Opus 4.6 --- commands/gameplay/scorebug.py | 67 ++++------ commands/gameplay/scorecard_tracker.py | 28 ++-- services/scorebug_service.py | 68 ++++++++-- tasks/live_scorebug_tracker.py | 91 ++++++++----- tests/test_scorebug_bugs.py | 176 +++++++++++++++++++++++++ utils/scorebug_helpers.py | 167 ++++++++++++----------- 6 files changed, 423 insertions(+), 174 deletions(-) create mode 100644 tests/test_scorebug_bugs.py diff --git a/commands/gameplay/scorebug.py b/commands/gameplay/scorebug.py index 24b996a..dee4780 100644 --- a/commands/gameplay/scorebug.py +++ b/commands/gameplay/scorebug.py @@ -3,6 +3,7 @@ Scorebug Commands Implements commands for publishing and displaying live game scorebugs from Google Sheets scorecards. """ + import discord from discord.ext import commands from discord import app_commands @@ -23,25 +24,21 @@ class ScorebugCommands(commands.Cog): def __init__(self, bot: commands.Bot): self.bot = bot - self.logger = get_contextual_logger(f'{__name__}.ScorebugCommands') + self.logger = get_contextual_logger(f"{__name__}.ScorebugCommands") self.scorebug_service = ScorebugService() self.scorecard_tracker = ScorecardTracker() self.logger.info("ScorebugCommands cog initialized") @app_commands.command( name="publish-scorecard", - description="Publish a Google Sheets scorecard to this channel for live tracking" + description="Publish a Google Sheets scorecard to this channel for live tracking", ) @app_commands.describe( url="Full URL to the Google Sheets scorecard or just the sheet key" ) @league_only() @logged_command("/publish-scorecard") - async def publish_scorecard( - self, - interaction: discord.Interaction, - url: str - ): + async def publish_scorecard(self, interaction: discord.Interaction, url: str): """ Link a Google Sheets scorecard to the current channel for live scorebug tracking. @@ -61,14 +58,16 @@ class ScorebugCommands(commands.Cog): # Verify it has a Scorebug tab try: - scorebug_data = await self.scorebug_service.read_scorebug_data(url, full_length=False) + scorebug_data = await self.scorebug_service.read_scorebug_data( + url, full_length=False + ) except SheetsException: embed = EmbedTemplate.error( title="Invalid Scorecard", description=( "This doesn't appear to be a valid scorecard.\n\n" "Make sure the sheet has a 'Scorebug' tab and is properly set up." - ) + ), ) await interaction.edit_original_response(content=None, embed=embed) return @@ -88,22 +87,22 @@ class ScorebugCommands(commands.Cog): # Store the scorecard in the tracker self.scorecard_tracker.publish_scorecard( - text_channel_id=interaction.channel_id, # type: ignore + text_channel_id=interaction.channel_id, # type: ignore sheet_url=url, - publisher_id=interaction.user.id + publisher_id=interaction.user.id, ) # Create success embed embed = EmbedTemplate.success( title="Scorecard Published", description=( - f"Your scorecard has been published to {interaction.channel.mention}\n\n" # type: ignore + f"Your scorecard has been published to {interaction.channel.mention}\n\n" # type: ignore f"**Sheet:** {scorecard.title}\n" f"**Status:** Live tracking enabled\n" f"**Scorecard:** {scorecard_link}\n\n" f"Anyone can now run `/scorebug` in this channel to see the current score.\n" f"The scorebug will also update in the live scores channel every 3 minutes." - ) + ), ) embed.add_field( @@ -112,7 +111,7 @@ class ScorebugCommands(commands.Cog): "`/scorebug` - Display full scorebug with details\n" "`/scorebug full_length:False` - Display compact scorebug" ), - inline=False + inline=False, ) await interaction.edit_original_response(content=None, embed=embed) @@ -121,13 +120,14 @@ class ScorebugCommands(commands.Cog): embed = EmbedTemplate.error( title="Cannot Access Scorecard", description=( - f"❌ {str(e)}\n\n" + f"{str(e)}\n\n" + f"**You provided:** `{url}`\n\n" f"**Common issues:**\n" f"• Sheet is not publicly accessible\n" f"• Invalid sheet URL or key\n" f"• Sheet doesn't exist\n\n" f"Make sure your sheet is shared with 'Anyone with the link can view'." - ) + ), ) await interaction.edit_original_response(content=None, embed=embed) @@ -138,23 +138,18 @@ class ScorebugCommands(commands.Cog): description=( "❌ An unexpected error occurred while publishing the scorecard.\n\n" "Please try again or contact support if the issue persists." - ) + ), ) await interaction.edit_original_response(content=None, embed=embed) @app_commands.command( - name="scorebug", - description="Display the scorebug for the game in this channel" - ) - @app_commands.describe( - full_length="Include full game details (defaults to True)" + name="scorebug", description="Display the scorebug for the game in this channel" ) + @app_commands.describe(full_length="Include full game details (defaults to True)") @league_only() @logged_command("/scorebug") async def scorebug( - self, - interaction: discord.Interaction, - full_length: bool = True + self, interaction: discord.Interaction, full_length: bool = True ): """ Display the current scorebug from the scorecard published in this channel. @@ -162,7 +157,7 @@ class ScorebugCommands(commands.Cog): await interaction.response.defer(ephemeral=True) # Check if a scorecard is published in this channel - sheet_url = self.scorecard_tracker.get_scorecard(interaction.channel_id) # type: ignore + sheet_url = self.scorecard_tracker.get_scorecard(interaction.channel_id) # type: ignore if not sheet_url: embed = EmbedTemplate.error( @@ -170,20 +165,17 @@ class ScorebugCommands(commands.Cog): description=( "❌ No scorecard has been published in this channel.\n\n" "Use `/publish-scorecard ` to publish a scorecard first." - ) + ), ) await interaction.followup.send(embed=embed, ephemeral=True) return try: # Read scorebug data - await interaction.edit_original_response( - content="📊 Reading scorebug..." - ) + await interaction.edit_original_response(content="📊 Reading scorebug...") scorebug_data = await self.scorebug_service.read_scorebug_data( - sheet_url, - full_length=full_length + sheet_url, full_length=full_length ) # Get team data @@ -196,16 +188,13 @@ class ScorebugCommands(commands.Cog): # Create scorebug embed using shared utility embed = create_scorebug_embed( - scorebug_data, - away_team, - home_team, - full_length + scorebug_data, away_team, home_team, full_length ) await interaction.edit_original_response(content=None, embed=embed) # Update timestamp in tracker - self.scorecard_tracker.update_timestamp(interaction.channel_id) # type: ignore + self.scorecard_tracker.update_timestamp(interaction.channel_id) # type: ignore except SheetsException as e: embed = EmbedTemplate.error( @@ -213,7 +202,7 @@ class ScorebugCommands(commands.Cog): description=( f"❌ {str(e)}\n\n" f"The scorecard may have been deleted or the sheet structure changed." - ) + ), ) await interaction.edit_original_response(content=None, embed=embed) @@ -224,7 +213,7 @@ class ScorebugCommands(commands.Cog): description=( "❌ An error occurred while reading the scorebug.\n\n" "Please try again or republish the scorecard." - ) + ), ) await interaction.edit_original_response(content=None, embed=embed) diff --git a/commands/gameplay/scorecard_tracker.py b/commands/gameplay/scorecard_tracker.py index 533a977..8b2a674 100644 --- a/commands/gameplay/scorecard_tracker.py +++ b/commands/gameplay/scorecard_tracker.py @@ -3,13 +3,14 @@ Scorecard Tracker Provides persistent tracking of published scorecards per Discord text channel using JSON file storage. """ + import json import logging from datetime import datetime, UTC from pathlib import Path from typing import Dict, List, Optional, Tuple -logger = logging.getLogger(f'{__name__}.ScorecardTracker') +logger = logging.getLogger(f"{__name__}.ScorecardTracker") class ScorecardTracker: @@ -39,9 +40,11 @@ class ScorecardTracker: """Load scorecard data from JSON file.""" try: if self.data_file.exists(): - with open(self.data_file, 'r') as f: + with open(self.data_file, "r") as f: self._data = json.load(f) - logger.debug(f"Loaded {len(self._data.get('scorecards', {}))} tracked scorecards") + logger.debug( + f"Loaded {len(self._data.get('scorecards', {}))} tracked scorecards" + ) else: self._data = {"scorecards": {}} logger.info("No existing scorecard data found, starting fresh") @@ -52,17 +55,14 @@ class ScorecardTracker: def save_data(self) -> None: """Save scorecard data to JSON file.""" try: - with open(self.data_file, 'w') as f: + with open(self.data_file, "w") as f: json.dump(self._data, f, indent=2, default=str) logger.debug("Scorecard data saved successfully") except Exception as e: logger.error(f"Failed to save scorecard data: {e}") def publish_scorecard( - self, - text_channel_id: int, - sheet_url: str, - publisher_id: int + self, text_channel_id: int, sheet_url: str, publisher_id: int ) -> None: """ Link a scorecard to a text channel. @@ -77,7 +77,7 @@ class ScorecardTracker: "sheet_url": sheet_url, "published_at": datetime.now(UTC).isoformat(), "last_updated": datetime.now(UTC).isoformat(), - "publisher_id": str(publisher_id) + "publisher_id": str(publisher_id), } self.save_data() logger.info(f"Published scorecard to channel {text_channel_id}: {sheet_url}") @@ -113,6 +113,7 @@ class ScorecardTracker: Returns: Sheet URL if published, None otherwise """ + self.load_data() scorecards = self._data.get("scorecards", {}) scorecard_data = scorecards.get(str(text_channel_id)) return scorecard_data["sheet_url"] if scorecard_data else None @@ -124,6 +125,7 @@ class ScorecardTracker: Returns: List of (text_channel_id, sheet_url) tuples """ + self.load_data() scorecards = self._data.get("scorecards", {}) return [ (int(channel_id), data["sheet_url"]) @@ -163,13 +165,17 @@ class ScorecardTracker: if channel_id not in valid_channel_ids: stale_entries.append(channel_id_str) except (ValueError, TypeError): - logger.warning(f"Invalid channel ID in scorecard data: {channel_id_str}") + logger.warning( + f"Invalid channel ID in scorecard data: {channel_id_str}" + ) stale_entries.append(channel_id_str) # Remove stale entries for channel_id_str in stale_entries: del scorecards[channel_id_str] - logger.info(f"Removed stale scorecard entry for channel ID: {channel_id_str}") + logger.info( + f"Removed stale scorecard entry for channel ID: {channel_id_str}" + ) if stale_entries: self.save_data() diff --git a/services/scorebug_service.py b/services/scorebug_service.py index f19494f..3f8adae 100644 --- a/services/scorebug_service.py +++ b/services/scorebug_service.py @@ -26,7 +26,7 @@ class ScorebugData: self.inning = data.get("inning", 1) self.is_final = data.get("is_final", False) self.outs = data.get("outs", 0) - self.win_percentage = data.get("win_percentage", 50.0) + self.win_percentage = data.get("win_percentage") # Current matchup information self.pitcher_name = data.get("pitcher_name", "") @@ -315,22 +315,70 @@ class ScorebugService(SheetsService): in_hole_url = matchups[3][1] if len(matchups[3]) > 1 else "" self.logger.debug(f" In Hole: {in_hole_name}") - # Parse win percentage from all_data[6][2] (Sheet D8 - row 8, column D) - self.logger.debug(f"📈 Parsing win percentage from D8 (all_data[6][2]):") + # Parse win percentage from C8 (team abbrev) and D8 (percentage) + # C8 = all_data[6][1] = winning team abbreviation + # D8 = all_data[6][2] = win probability percentage + # The sheet outputs the LEADING team's win%, so we need to + # normalize to home team's win% for the progress bar. + self.logger.debug( + f"📈 Parsing win percentage from C8:D8 (all_data[6][1:3]):" + ) try: + win_pct_team_raw = ( + all_data[6][1] + if len(all_data) > 6 and len(all_data[6]) > 1 + else None + ) win_pct_raw = ( all_data[6][2] if len(all_data) > 6 and len(all_data[6]) > 2 - else "50%" + else None ) + self.logger.debug(f" Raw win percentage team: '{win_pct_team_raw}'") self.logger.debug(f" Raw win percentage value: '{win_pct_raw}'") - # Remove % sign if present and convert to float - win_pct_str = str(win_pct_raw).replace("%", "").strip() - win_percentage = float(win_pct_str) if win_pct_str else 50.0 - self.logger.debug(f" ✅ Parsed win percentage: {win_percentage}%") + + if win_pct_raw is None or str(win_pct_raw).strip() == "": + self.logger.info( + f" Win percentage unavailable (raw value: '{win_pct_raw}')" + ) + win_percentage = None + else: + # Remove % sign if present and convert to float + win_pct_str = str(win_pct_raw).replace("%", "").strip() + win_percentage = float(win_pct_str) + + # Handle 0.0-1.0 range (pygsheets may return decimal like 0.75) + if 0.0 <= win_percentage <= 1.0: + win_percentage = win_percentage * 100 + + # The sheet gives the LEADING team's win%. + # Progress bar expects HOME team's win%. + # Compare C8 abbreviation to home team abbreviation to orient correctly. + home_abbrev_raw = ( + game_state[4][1] + if len(game_state) > 4 and len(game_state[4]) > 1 + else "" + ) + win_pct_team = ( + str(win_pct_team_raw).strip() if win_pct_team_raw else "" + ) + + if win_pct_team and win_pct_team != home_abbrev_raw: + # The percentage belongs to the away team, flip for home perspective + self.logger.debug( + f" Win% team '{win_pct_team}' is away (home is '{home_abbrev_raw}'), " + f"flipping {win_percentage}% -> {100 - win_percentage}%" + ) + win_percentage = 100 - win_percentage + + self.logger.debug( + f" ✅ Parsed home win percentage: {win_percentage}%" + ) except (ValueError, IndexError, AttributeError) as e: - self.logger.warning(f" ⚠️ Failed to parse win percentage: {e}") - win_percentage = 50.0 + self.logger.info( + f" Win percentage could not be parsed (raw value: '{win_pct_raw}'): {e}" + ) + win_percentage = None self.logger.debug(f"📊 Final parsed values:") self.logger.debug(f" Away team {away_team_id}: {away_score}") diff --git a/tasks/live_scorebug_tracker.py b/tasks/live_scorebug_tracker.py index 5f76269..9013ac2 100644 --- a/tasks/live_scorebug_tracker.py +++ b/tasks/live_scorebug_tracker.py @@ -3,6 +3,7 @@ Live Scorebug Tracker Background task that monitors published scorecards and updates live score displays. """ + import asyncio from typing import List import discord @@ -39,7 +40,7 @@ class LiveScorebugTracker: bot: Discord bot instance """ self.bot = bot - self.logger = get_contextual_logger(f'{__name__}.LiveScorebugTracker') + self.logger = get_contextual_logger(f"{__name__}.LiveScorebugTracker") self.scorebug_service = ScorebugService() self.scorecard_tracker = ScorecardTracker() self.voice_tracker = VoiceChannelTracker() @@ -83,10 +84,14 @@ class LiveScorebugTracker: return # Get live scores channel - live_scores_channel = discord.utils.get(guild.text_channels, name='live-sba-scores') + live_scores_channel = discord.utils.get( + guild.text_channels, name="live-sba-scores" + ) if not live_scores_channel: - self.logger.warning("live-sba-scores channel not found, skipping channel update") + self.logger.warning( + "live-sba-scores channel not found, skipping channel update" + ) # Don't return - still update voice channels else: # Get all published scorecards @@ -96,76 +101,84 @@ class LiveScorebugTracker: # No active scorebugs - clear the channel and hide it await self._clear_live_scores_channel(live_scores_channel) await set_channel_visibility( - live_scores_channel, - visible=False, - reason="No active games" + live_scores_channel, visible=False, reason="No active games" ) return # Read all scorebugs and create embeds active_scorebugs = [] + read_failures = 0 + confirmed_final = 0 for text_channel_id, sheet_url in all_scorecards: try: scorebug_data = await self.scorebug_service.read_scorebug_data( - sheet_url, - full_length=False # Compact view for live channel + sheet_url, full_length=False # Compact view for live channel ) # Only include active (non-final) games if scorebug_data.is_active: # Get team data - away_team = await team_service.get_team(scorebug_data.away_team_id) - home_team = await team_service.get_team(scorebug_data.home_team_id) + away_team = await team_service.get_team( + scorebug_data.away_team_id + ) + home_team = await team_service.get_team( + scorebug_data.home_team_id + ) if away_team is None or home_team is None: - raise ValueError(f'Error looking up teams in scorecard; IDs provided: {scorebug_data.away_team_id} & {scorebug_data.home_team_id}') + raise ValueError( + f"Error looking up teams in scorecard; IDs provided: {scorebug_data.away_team_id} & {scorebug_data.home_team_id}" + ) # Create compact embed using shared utility embed = create_scorebug_embed( scorebug_data, away_team, home_team, - full_length=False # Compact view for live channel + full_length=False, # Compact view for live channel ) active_scorebugs.append(embed) # Update associated voice channel if it exists await self._update_voice_channel_description( - text_channel_id, - scorebug_data, - away_team, - home_team + text_channel_id, scorebug_data, away_team, home_team ) + else: + confirmed_final += 1 await asyncio.sleep(1) # Rate limit between reads except SheetsException as e: + read_failures += 1 self.logger.warning(f"Could not read scorecard {sheet_url}: {e}") except Exception as e: + read_failures += 1 self.logger.error(f"Error processing scorecard {sheet_url}: {e}") # Update live scores channel if active_scorebugs: await set_channel_visibility( - live_scores_channel, - visible=True, - reason="Active games in progress" + live_scores_channel, visible=True, reason="Active games in progress" + ) + await self._post_scorebugs_to_channel( + live_scores_channel, active_scorebugs + ) + elif read_failures > 0 and confirmed_final < len(all_scorecards): + # Some reads failed — don't hide the channel, preserve last state + self.logger.warning( + f"Skipping channel hide: {read_failures} scorecard read(s) failed, " + f"only {confirmed_final}/{len(all_scorecards)} confirmed final" ) - await self._post_scorebugs_to_channel(live_scores_channel, active_scorebugs) else: - # All games finished - clear the channel and hide it + # All games confirmed final — safe to clear and hide await self._clear_live_scores_channel(live_scores_channel) await set_channel_visibility( - live_scores_channel, - visible=False, - reason="No active games" + live_scores_channel, visible=False, reason="No active games" ) async def _post_scorebugs_to_channel( - self, - channel: discord.TextChannel, - embeds: List[discord.Embed] + self, channel: discord.TextChannel, embeds: List[discord.Embed] ): """ Post scorebugs to the live scores channel. @@ -185,7 +198,7 @@ class LiveScorebugTracker: else: # Split into multiple messages if more than 10 embeds for i in range(0, len(embeds), 10): - batch = embeds[i:i+10] + batch = embeds[i : i + 10] await channel.send(embeds=batch) self.logger.info(f"Posted {len(embeds)} scorebugs to live-sba-scores") @@ -219,7 +232,7 @@ class LiveScorebugTracker: text_channel_id: int, scorebug_data: ScorebugData, away_team: Team, - home_team: Team + home_team: Team, ): """ Update voice channel description with live score. @@ -232,10 +245,14 @@ class LiveScorebugTracker: """ try: # Check if there's an associated voice channel - voice_channel_id = self.voice_tracker.get_voice_channel_for_text_channel(text_channel_id) + voice_channel_id = self.voice_tracker.get_voice_channel_for_text_channel( + text_channel_id + ) if not voice_channel_id: - self.logger.debug(f'No voice channel associated with text channel ID {text_channel_id} (may have been cleaned up)') + self.logger.debug( + f"No voice channel associated with text channel ID {text_channel_id} (may have been cleaned up)" + ) return # No associated voice channel # Get the voice channel @@ -248,7 +265,9 @@ class LiveScorebugTracker: voice_channel = guild.get_channel(voice_channel_id) if not voice_channel or not isinstance(voice_channel, discord.VoiceChannel): - self.logger.debug(f"Voice channel {voice_channel_id} not found or wrong type") + self.logger.debug( + f"Voice channel {voice_channel_id} not found or wrong type" + ) return # Format description: "BOS 4 @ 3 NYY" or "BOS 5 @ 3 NYY - FINAL" @@ -263,10 +282,14 @@ class LiveScorebugTracker: # Update voice channel description (topic) await voice_channel.edit(status=description) - self.logger.debug(f"Updated voice channel {voice_channel.name} description to: {description}") + self.logger.debug( + f"Updated voice channel {voice_channel.name} description to: {description}" + ) except discord.Forbidden: - self.logger.warning(f"Missing permissions to update voice channel {voice_channel_id}") + self.logger.warning( + f"Missing permissions to update voice channel {voice_channel_id}" + ) except Exception as e: self.logger.error(f"Error updating voice channel description: {e}") diff --git a/tests/test_scorebug_bugs.py b/tests/test_scorebug_bugs.py new file mode 100644 index 0000000..c36023b --- /dev/null +++ b/tests/test_scorebug_bugs.py @@ -0,0 +1,176 @@ +""" +Tests for scorebug bug fixes (#39 and #40). + +#40: ScorecardTracker reads stale in-memory data — fix ensures get_scorecard() + and get_all_scorecards() reload from disk before returning data. + +#39: Win percentage stuck at 50% — fix makes parsing robust for decimal (0.75), + percentage string ("75%"), plain number ("75"), empty, and None values. + When parsing fails, win_percentage is None instead of a misleading 50.0. +""" + +import json +import tempfile +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from commands.gameplay.scorecard_tracker import ScorecardTracker +from services.scorebug_service import ScorebugData +from utils.scorebug_helpers import create_scorebug_embed, create_team_progress_bar + + +class TestScorecardTrackerFreshReads: + """Tests that ScorecardTracker reads fresh data from disk (fix for #40).""" + + def test_get_all_scorecards_reads_fresh_data(self, tmp_path): + """get_all_scorecards() should pick up scorecards written by another process. + + Simulates the background task having a stale tracker instance while + the /publish-scorecard command writes new data to the JSON file. + """ + data_file = tmp_path / "scorecards.json" + data_file.write_text(json.dumps({"scorecards": {}})) + + tracker = ScorecardTracker(data_file=str(data_file)) + assert tracker.get_all_scorecards() == [] + + # Another process writes a scorecard to the same file + new_data = { + "scorecards": { + "111": { + "text_channel_id": "111", + "sheet_url": "https://docs.google.com/spreadsheets/d/abc123", + "published_at": "2026-01-01T00:00:00", + "last_updated": "2026-01-01T00:00:00", + "publisher_id": "999", + } + } + } + data_file.write_text(json.dumps(new_data)) + + # Should see the new scorecard without restart + result = tracker.get_all_scorecards() + assert len(result) == 1 + assert result[0] == (111, "https://docs.google.com/spreadsheets/d/abc123") + + def test_get_scorecard_reads_fresh_data(self, tmp_path): + """get_scorecard() should pick up a scorecard written by another process.""" + data_file = tmp_path / "scorecards.json" + data_file.write_text(json.dumps({"scorecards": {}})) + + tracker = ScorecardTracker(data_file=str(data_file)) + assert tracker.get_scorecard(222) is None + + # Another process writes a scorecard + new_data = { + "scorecards": { + "222": { + "text_channel_id": "222", + "sheet_url": "https://docs.google.com/spreadsheets/d/xyz789", + "published_at": "2026-01-01T00:00:00", + "last_updated": "2026-01-01T00:00:00", + "publisher_id": "999", + } + } + } + data_file.write_text(json.dumps(new_data)) + + # Should see the new scorecard + assert ( + tracker.get_scorecard(222) + == "https://docs.google.com/spreadsheets/d/xyz789" + ) + + +class TestWinPercentageParsing: + """Tests for robust win percentage parsing in ScorebugData (fix for #39).""" + + def test_percentage_string(self): + """'75%' string should parse to 75.0.""" + data = ScorebugData({"win_percentage": 75.0}) + assert data.win_percentage == 75.0 + + def test_none_default(self): + """Missing win_percentage key should default to None.""" + data = ScorebugData({}) + assert data.win_percentage is None + + def test_explicit_none(self): + """Explicit None should stay None.""" + data = ScorebugData({"win_percentage": None}) + assert data.win_percentage is None + + def test_zero_is_valid(self): + """0.0 win percentage is a valid value (team certain to lose).""" + data = ScorebugData({"win_percentage": 0.0}) + assert data.win_percentage == 0.0 + + +class TestWinPercentageEmbed: + """Tests for embed creation with win_percentage=None (fix for #39 Part B).""" + + def _make_scorebug_data(self, win_percentage): + """Create minimal ScorebugData for embed testing.""" + return ScorebugData( + { + "away_team_id": 1, + "home_team_id": 2, + "header": "Test Game", + "away_score": 10, + "home_score": 2, + "which_half": "Top", + "inning": 5, + "is_final": False, + "outs": 1, + "win_percentage": win_percentage, + "pitcher_name": "", + "batter_name": "", + "runners": [["", ""], ["", ""], ["", ""], ["", ""]], + "summary": [], + } + ) + + def _make_team(self, abbrev, color_int=0x3498DB): + """Create a mock team object.""" + team = MagicMock() + team.abbrev = abbrev + team.get_color_int.return_value = color_int + return team + + def test_embed_with_none_win_percentage_shows_unavailable(self): + """When win_percentage is None, embed should show unavailable message.""" + data = self._make_scorebug_data(win_percentage=None) + away = self._make_team("POR") + home = self._make_team("WV") + + embed = create_scorebug_embed(data, away, home, full_length=False) + + # Find the Win Probability field + win_prob_field = next(f for f in embed.fields if f.name == "Win Probability") + assert "unavailable" in win_prob_field.value.lower() + + def test_embed_with_valid_win_percentage_shows_bar(self): + """When win_percentage is valid, embed should show the progress bar.""" + data = self._make_scorebug_data(win_percentage=75.0) + away = self._make_team("POR") + home = self._make_team("WV") + + embed = create_scorebug_embed(data, away, home, full_length=False) + + win_prob_field = next(f for f in embed.fields if f.name == "Win Probability") + assert "75.0%" in win_prob_field.value + assert "unavailable" not in win_prob_field.value.lower() + + def test_embed_with_50_percent_shows_even_bar(self): + """50% win probability should show the even/balanced bar.""" + data = self._make_scorebug_data(win_percentage=50.0) + away = self._make_team("POR") + home = self._make_team("WV") + + embed = create_scorebug_embed(data, away, home, full_length=False) + + win_prob_field = next(f for f in embed.fields if f.name == "Win Probability") + assert "50.0%" in win_prob_field.value + assert "=" in win_prob_field.value diff --git a/utils/scorebug_helpers.py b/utils/scorebug_helpers.py index 325557c..14e763b 100644 --- a/utils/scorebug_helpers.py +++ b/utils/scorebug_helpers.py @@ -3,16 +3,14 @@ Scorebug Display Helpers Utility functions for formatting and displaying scorebug information. """ + import discord from views.embeds import EmbedColors def create_scorebug_embed( - scorebug_data, - away_team, - home_team, - full_length: bool = True + scorebug_data, away_team, home_team, full_length: bool = True ) -> discord.Embed: """ Create a rich embed from scorebug data. @@ -29,66 +27,86 @@ def create_scorebug_embed( # Determine embed color based on win probability (not score!) # This creates a fun twist where the favored team's color shows, # even if they're currently losing - if scorebug_data.win_percentage > 50 and home_team: + if ( + scorebug_data.win_percentage is not None + and scorebug_data.win_percentage > 50 + and home_team + ): embed_color = home_team.get_color_int() # Home team favored - elif scorebug_data.win_percentage < 50 and away_team: + elif ( + scorebug_data.win_percentage is not None + and scorebug_data.win_percentage < 50 + and away_team + ): embed_color = away_team.get_color_int() # Away team favored else: - embed_color = EmbedColors.INFO # Even game (50/50) + embed_color = EmbedColors.INFO # Even game (50/50) or unavailable # Create embed with header as title - embed = discord.Embed( - title=scorebug_data.header, - color=embed_color - ) + embed = discord.Embed(title=scorebug_data.header, color=embed_color) # Get team abbreviations for use throughout away_abbrev = away_team.abbrev if away_team else "AWAY" home_abbrev = home_team.abbrev if home_team else "HOME" # Create ASCII scorebug with bases visualization - occupied = '●' - unoccupied = '○' + occupied = "●" + unoccupied = "○" # runners[0]=Catcher, [1]=On First, [2]=On Second, [3]=On Third - first_base = unoccupied if not scorebug_data.runners[1] or not scorebug_data.runners[1][0] else occupied - second_base = unoccupied if not scorebug_data.runners[2] or not scorebug_data.runners[2][0] else occupied - third_base = unoccupied if not scorebug_data.runners[3] or not scorebug_data.runners[3][0] else occupied - half = '▲' if scorebug_data.which_half == 'Top' else '▼' + first_base = ( + unoccupied + if not scorebug_data.runners[1] or not scorebug_data.runners[1][0] + else occupied + ) + second_base = ( + unoccupied + if not scorebug_data.runners[2] or not scorebug_data.runners[2][0] + else occupied + ) + third_base = ( + unoccupied + if not scorebug_data.runners[3] or not scorebug_data.runners[3][0] + else occupied + ) + half = "▲" if scorebug_data.which_half == "Top" else "▼" if not scorebug_data.is_final: - inning = f'{half} {scorebug_data.inning}' + inning = f"{half} {scorebug_data.inning}" outs = f'{scorebug_data.outs} Out{"s" if scorebug_data.outs != 1 else ""}' else: # Final inning display - final_inning = scorebug_data.inning if scorebug_data.which_half == "Bot" else scorebug_data.inning - 1 - inning = f'F/{final_inning}' - outs = '' + final_inning = ( + scorebug_data.inning + if scorebug_data.which_half == "Bot" + else scorebug_data.inning - 1 + ) + inning = f"F/{final_inning}" + outs = "" game_state_text = ( - f'```\n' - f'{away_abbrev: ^4}{scorebug_data.away_score: ^3} {second_base}{inning: >10}\n' - f'{home_abbrev: ^4}{scorebug_data.home_score: ^3} {third_base} {first_base}{outs: >8}\n' - f'```' + f"```\n" + f"{away_abbrev: ^4}{scorebug_data.away_score: ^3} {second_base}{inning: >10}\n" + f"{home_abbrev: ^4}{scorebug_data.home_score: ^3} {third_base} {first_base}{outs: >8}\n" + f"```" ) - # Add win probability bar + # Add win probability bar or unavailable message + if scorebug_data.win_percentage is not None: + win_prob_value = create_team_progress_bar( + scorebug_data.win_percentage, away_abbrev, home_abbrev + ) + else: + win_prob_value = "-# Win probability unavailable" + embed.add_field( - name='Win Probability', - value=create_team_progress_bar( - scorebug_data.win_percentage, - away_abbrev, - home_abbrev - ), - inline=False + name="Win Probability", + value=win_prob_value, + inline=False, ) # Add game state - embed.add_field( - name='Game State', - value=game_state_text, - inline=False - ) + embed.add_field(name="Game State", value=game_state_text, inline=False) # If not full_length, return compact version if not full_length: @@ -97,70 +115,59 @@ def create_scorebug_embed( # Full length - add pitcher and batter info if scorebug_data.pitcher_name: embed.add_field( - name='Pitcher', - value=f'[{scorebug_data.pitcher_name}]({scorebug_data.pitcher_url})\n{scorebug_data.pitcher_stats}', - inline=True + name="Pitcher", + value=f"[{scorebug_data.pitcher_name}]({scorebug_data.pitcher_url})\n{scorebug_data.pitcher_stats}", + inline=True, ) if scorebug_data.batter_name: embed.add_field( - name='Batter', - value=f'[{scorebug_data.batter_name}]({scorebug_data.batter_url})\n{scorebug_data.batter_stats}', - inline=True + name="Batter", + value=f"[{scorebug_data.batter_name}]({scorebug_data.batter_url})\n{scorebug_data.batter_stats}", + inline=True, ) # Add baserunners if present - on_first = scorebug_data.runners[1] if scorebug_data.runners[1] else '' - on_second = scorebug_data.runners[2] if scorebug_data.runners[2] else '' - on_third = scorebug_data.runners[3] if scorebug_data.runners[3] else '' + on_first = scorebug_data.runners[1] if scorebug_data.runners[1] else "" + on_second = scorebug_data.runners[2] if scorebug_data.runners[2] else "" + on_third = scorebug_data.runners[3] if scorebug_data.runners[3] else "" have_baserunners = len(on_first[0]) + len(on_second[0]) + len(on_third[0]) > 0 if have_baserunners > 0: - br_string = '' + br_string = "" if len(on_first) > 0: - br_string += f'On First: [{on_first[0]}]({on_first[1]})\n' + br_string += f"On First: [{on_first[0]}]({on_first[1]})\n" if len(on_second) > 0: - br_string += f'On Second: [{on_second[0]}]({on_second[1]})\n' + br_string += f"On Second: [{on_second[0]}]({on_second[1]})\n" if len(on_third) > 0: - br_string += f'On Third: [{on_third[0]}]({on_third[1]})\n' + br_string += f"On Third: [{on_third[0]}]({on_third[1]})\n" - embed.add_field(name=' ', value=' ', inline=False) # Spacer - embed.add_field( - name='Baserunners', - value=br_string, - inline=True - ) + embed.add_field(name=" ", value=" ", inline=False) # Spacer + embed.add_field(name="Baserunners", value=br_string, inline=True) # Add catcher if scorebug_data.runners[0] and scorebug_data.runners[0][0]: embed.add_field( - name='Catcher', - value=f'[{scorebug_data.runners[0][0]}]({scorebug_data.runners[0][1]})', - inline=True + name="Catcher", + value=f"[{scorebug_data.runners[0][0]}]({scorebug_data.runners[0][1]})", + inline=True, ) # Add inning summary if not final if not scorebug_data.is_final and scorebug_data.summary: - i_string = '' + i_string = "" for line in scorebug_data.summary: if line and len(line) >= 2 and line[0]: - i_string += f'- Play {line[0]}: {line[1]}\n' + i_string += f"- Play {line[0]}: {line[1]}\n" if i_string and "IFERROR" not in i_string: - embed.add_field( - name='Inning Summary', - value=i_string, - inline=False - ) + embed.add_field(name="Inning Summary", value=i_string, inline=False) return embed def create_team_progress_bar( - win_percentage: float, - away_abbrev: str, - home_abbrev: str, - length: int = 10 + win_percentage: float, away_abbrev: str, home_abbrev: str, length: int = 10 ) -> str: """ Create a proportional progress bar showing each team's win probability. @@ -186,23 +193,23 @@ def create_team_progress_bar( if win_percentage > 50: # Home team (right side) is winning - away_char = '░' # Light blocks for losing team - home_char = '▓' # Dark blocks for winning team + away_char = "░" # Light blocks for losing team + home_char = "▓" # Dark blocks for winning team bar = away_char * away_blocks + home_char * home_blocks # Arrow extends from right side, percentage on right - return f'{away_abbrev} {bar}► {home_abbrev} {win_percentage:.1f}%' + return f"{away_abbrev} {bar}► {home_abbrev} {win_percentage:.1f}%" elif win_percentage < 50: # Away team (left side) is winning - away_char = '▓' # Dark blocks for winning team - home_char = '░' # Light blocks for losing team + away_char = "▓" # Dark blocks for winning team + home_char = "░" # Light blocks for losing team bar = away_char * away_blocks + home_char * home_blocks # Arrow extends from left side, percentage on left (showing away team's win %) away_win_pct = 100 - win_percentage - return f'{away_win_pct:.1f}% {away_abbrev} ◄{bar} {home_abbrev}' + return f"{away_win_pct:.1f}% {away_abbrev} ◄{bar} {home_abbrev}" else: # Even game (50/50) - away_char = '▓' - home_char = '▓' + away_char = "▓" + home_char = "▓" bar = away_char * away_blocks + home_char * home_blocks # Arrows on both sides for balanced display, percentage on both sides - return f'{win_percentage:.1f}% {away_abbrev} ={bar}= {home_abbrev} {win_percentage:.1f}%' + return f"{win_percentage:.1f}% {away_abbrev} ={bar}= {home_abbrev} {win_percentage:.1f}%"