major-domo-v2/commands/README.md
Cal Corum 0c6f7c8ffe CLAUDE: Fix GroupCog interaction bug and GIF display issues
This commit addresses critical bugs in the injury command system and
establishes best practices for Discord command groups.

## Critical Fixes

### 1. GroupCog → app_commands.Group Migration
- **Problem**: `commands.GroupCog` has a duplicate interaction processing bug
  causing "404 Unknown interaction" errors when deferring responses
- **Root Cause**: GroupCog triggers command handler twice, consuming the
  interaction token before the second execution can respond
- **Solution**: Migrated InjuryCog to InjuryGroup using `app_commands.Group`
  pattern (same as ChartManageGroup and ChartCategoryGroup)
- **Result**: Reliable command execution, no more 404 errors

### 2. GiphyService GIF URL Fix
- **Problem**: Giphy service returned web page URLs (https://giphy.com/gifs/...)
  instead of direct image URLs, preventing Discord embed display
- **Root Cause**: Code accessed `data.url` instead of `data.images.original.url`
- **Solution**: Updated both `get_disappointment_gif()` and `get_gif()` methods
  to use correct API response path for embeddable GIF URLs
- **Result**: GIFs now display correctly in Discord embeds

## Documentation

### Command Groups Best Practices (commands/README.md)
Added comprehensive section documenting:
- **Critical Warning**: Never use `commands.GroupCog` - use `app_commands.Group`
- **Technical Explanation**: Why GroupCog fails (duplicate execution bug)
- **Migration Guide**: Step-by-step conversion from GroupCog to Group
- **Comparison Table**: Key differences between the two approaches
- **Working Examples**: References to ChartManageGroup, InjuryGroup patterns

## Architecture Changes

### Injury Commands (`commands/injuries/`)
- Converted from `commands.GroupCog` to `app_commands.Group`
- Registration via `bot.tree.add_command()` instead of `bot.add_cog()`
- Removed workarounds for GroupCog duplicate interaction issues
- Clean defer/response pattern with `@logged_command` decorator

### GiphyService (`services/giphy_service.py`)
- Centralized from `commands/soak/giphy_service.py`
- Now returns direct GIF image URLs for Discord embeds
- Maintains Trump GIF filtering (legacy behavior)
- Added gif_url to log output for debugging

### Configuration (`config.py`)
- Added `giphy_api_key` and `giphy_translate_url` settings
- Environment variable support via `GIPHY_API_KEY`
- Default values provided for out-of-box functionality

## Files Changed
- commands/injuries/: New InjuryGroup with app_commands.Group pattern
- services/giphy_service.py: Centralized service with GIF URL fix
- commands/soak/giphy_service.py: Backwards compatibility wrapper
- commands/README.md: Command groups best practices documentation
- config.py: Giphy configuration settings
- services/__init__.py: GiphyService exports

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-16 22:15:42 -05:00

19 KiB

Commands Package Documentation

Discord Bot v2.0 - Scalable Command Architecture

This document outlines the command architecture, patterns, and best practices established for the SBA Discord Bot v2.0.

📁 Architecture Overview

Package Structure

commands/
├── README.md                    # This documentation
├── __init__.py                  # Future: Global command utilities
└── players/                     # Player-related commands
    ├── __init__.py             # Package setup with resilient loading
    └── info.py                 # Player information commands

Future Expansion (Phase 2+)

commands/
├── README.md
├── __init__.py                  
├── players/                     # ✅ COMPLETED
│   ├── __init__.py
│   ├── info.py                 # /player command
│   ├── search.py               # /player-search, /player-lookup
│   ├── stats.py                # /player-stats, /player-compare
│   └── rankings.py             # /player-rankings, /leaderboard
├── teams/                       # 🔄 PLANNED
│   ├── __init__.py
│   ├── roster.py               # /team-roster, /team-depth
│   ├── stats.py                # /team-stats, /team-leaders
│   └── schedule.py             # /team-schedule, /team-record
├── league/                      # 🔄 PLANNED
│   ├── __init__.py
│   ├── standings.py            # /standings, /playoff-race
│   ├── schedule.py             # /schedule, /scores
│   └── leaders.py              # /leaders, /awards
├── draft/                       # 🔄 PLANNED
│   ├── __init__.py
│   ├── picks.py                # /draft-pick, /draft-order
│   ├── board.py                # /draft-board, /draft-list
│   └── timer.py                # /draft-status, /draft-timer
├── transactions/                # 🔄 PLANNED
│   ├── __init__.py
│   ├── trades.py               # /trade, /trade-history
│   ├── waivers.py              # /waivers, /free-agents
│   └── history.py              # /transaction-history
├── admin/                       # 🔄 PLANNED
│   ├── __init__.py
│   ├── league.py               # /admin-season, /admin-week
│   ├── draft.py                # /admin-draft, /admin-timer
│   └── system.py               # /health, /sync-commands
└── utils/                       # 🔄 PLANNED
    ├── __init__.py
    ├── dice.py                 # /roll, /dice
    └── fun.py                  # Fun/misc commands

🏗️ Design Principles

1. Single Responsibility

  • Each file handles 2-4 closely related commands
  • Clear logical grouping by domain (players, teams, etc.)
  • Focused functionality reduces complexity

2. Resilient Loading

  • One failed cog doesn't break the entire package
  • Loop-based loading with comprehensive error handling
  • Clear logging for debugging and monitoring

3. Scalable Architecture

  • Easy to add new packages and cogs
  • Consistent patterns across all command groups
  • Future-proof structure for bot growth

4. Modern Discord.py Patterns

  • Application commands (slash commands) only
  • Proper error handling with user-friendly messages
  • Async/await throughout
  • Type hints and comprehensive documentation

🔧 Implementation Patterns

Command Package Structure

Individual Command File (e.g., players/info.py)

"""
Player Information Commands

Implements slash commands for displaying player information and statistics.
"""
import logging
from typing import Optional

import discord
from discord.ext import commands

from services.player_service import player_service
from exceptions import BotException

logger = logging.getLogger(f'{__name__}.PlayerInfoCommands')


class PlayerInfoCommands(commands.Cog):
    """Player information and statistics command handlers."""
    
    def __init__(self, bot: commands.Bot):
        self.bot = bot
    
    @discord.app_commands.command(
        name="player",
        description="Display player information and statistics"
    )
    @discord.app_commands.describe(
        name="Player name to search for",
        season="Season to show stats for (defaults to current season)"
    )
    async def player_info(
        self,
        interaction: discord.Interaction,
        name: str,
        season: Optional[int] = None
    ):
        """Display player card with statistics."""
        try:
            # Always defer for potentially slow API calls
            await interaction.response.defer()
            
            # Command implementation here
            # Use logger for error logging
            # Create Discord embeds for responses
            
        except Exception as e:
            logger.error(f"Player info command error: {e}", exc_info=True)
            error_msg = "❌ Error retrieving player information."
            
            if interaction.response.is_done():
                await interaction.followup.send(error_msg, ephemeral=True)
            else:
                await interaction.response.send_message(error_msg, ephemeral=True)


async def setup(bot: commands.Bot):
    """Load the player info commands cog."""
    await bot.add_cog(PlayerInfoCommands(bot))

Package init.py with Resilient Loading

"""
Player Commands Package

This package contains all player-related Discord commands organized into focused modules.
"""
import logging
from discord.ext import commands

from .info import PlayerInfoCommands
# Future imports:
# from .search import PlayerSearchCommands
# from .stats import PlayerStatsCommands

logger = logging.getLogger(__name__)


async def setup_players(bot: commands.Bot):
    """
    Setup all player command modules.
    
    Returns:
        tuple: (successful_count, failed_count, failed_modules)
    """
    # Define all player command cogs to load
    player_cogs = [
        ("PlayerInfoCommands", PlayerInfoCommands),
        # Future cogs:
        # ("PlayerSearchCommands", PlayerSearchCommands),
        # ("PlayerStatsCommands", PlayerStatsCommands), 
    ]
    
    successful = 0
    failed = 0
    failed_modules = []
    
    for cog_name, cog_class in player_cogs:
        try:
            await bot.add_cog(cog_class(bot))
            logger.info(f"✅ Loaded {cog_name}")
            successful += 1
        except Exception as e:
            logger.error(f"❌ Failed to load {cog_name}: {e}", exc_info=True)
            failed += 1
            failed_modules.append(cog_name)
    
    # Log summary
    if failed == 0:
        logger.info(f"🎉 All {successful} player command modules loaded successfully")
    else:
        logger.warning(f"⚠️  Player commands loaded with issues: {successful} successful, {failed} failed")
    
    return successful, failed, failed_modules


# Export the setup function for easy importing
__all__ = ['setup_players', 'PlayerInfoCommands']

🔄 Smart Command Syncing

Hash-Based Change Detection

The bot implements smart command syncing that only updates Discord when commands actually change:

Development Mode:

  • Automatically detects command changes using SHA-256 hashing
  • Only syncs when changes are detected
  • Saves hash to .last_command_hash for comparison
  • Prevents unnecessary Discord API calls

Production Mode:

  • No automatic syncing
  • Commands must be manually synced using /sync command
  • Prevents accidental command updates in production

How It Works

  1. Hash Generation: Creates hash of command names, descriptions, and parameters
  2. Comparison: Compares current hash with stored hash from .last_command_hash
  3. Conditional Sync: Only syncs if hashes differ or no previous hash exists
  4. Hash Storage: Saves new hash after successful sync

Benefits

  • API Efficiency: Avoids Discord rate limits
  • Development Speed: Fast restarts when no command changes
  • Production Safety: No accidental command updates
  • Consistency: Commands stay consistent across restarts

🚀 Bot Integration

Command Loading in bot.py

async def setup_hook(self):
    """Called when the bot is starting up."""
    # Load command packages
    await self._load_command_packages()
    
    # Smart command syncing: auto-sync in development if changes detected
    config = get_config()
    if config.is_development:
        if await self._should_sync_commands():
            self.logger.info("Development mode: changes detected, syncing commands...")
            await self._sync_commands()
            await self._save_command_hash()
        else:
            self.logger.info("Development mode: no command changes detected, skipping sync")
    else:
        self.logger.info("Production mode: commands loaded but not auto-synced")

async def _load_command_packages(self):
    """Load all command packages with resilient error handling."""
    from commands.players import setup_players
    
    # Define command packages to load
    command_packages = [
        ("players", setup_players),
        # Future packages:
        # ("teams", setup_teams),
        # ("league", setup_league), 
    ]
    
    # Loop-based loading with error isolation
    for package_name, setup_func in command_packages:
        try:
            successful, failed, failed_modules = await setup_func(self)
            # Log results
        except Exception as e:
            self.logger.error(f"❌ Failed to load {package_name} package: {e}")

📋 Development Guidelines

Adding New Command Packages

1. Create Package Structure

mkdir commands/teams
touch commands/teams/__init__.py
touch commands/teams/roster.py

2. Implement Command Module

  • Follow the pattern from players/info.py
  • Use module-level logger: logger = logging.getLogger(f'{__name__}.ClassName')
  • Always defer responses: await interaction.response.defer()
  • Comprehensive error handling with user-friendly messages
  • Type hints and docstrings

3. Create Package Setup Function

  • Follow the pattern from players/__init__.py
  • Use loop-based cog loading with error isolation
  • Return tuple: (successful, failed, failed_modules)
  • Comprehensive logging with emojis for quick scanning

4. Register in Bot

  • Add import to _load_command_packages() in bot.py
  • Add to command_packages list
  • Test in development environment

Adding Commands to Existing Packages

1. Create New Command Module

# commands/players/search.py
class PlayerSearchCommands(commands.Cog):
    # Implementation
    pass

async def setup(bot: commands.Bot):
    await bot.add_cog(PlayerSearchCommands(bot))

2. Update Package init.py

from .search import PlayerSearchCommands

# Add to player_cogs list
player_cogs = [
    ("PlayerInfoCommands", PlayerInfoCommands),
    ("PlayerSearchCommands", PlayerSearchCommands),  # New cog
]

3. Test Import Structure

# Verify imports work
from commands.players import setup_players
from commands.players.search import PlayerSearchCommands

🎯 Best Practices

Command Implementation

  1. Always defer responses for API calls: await interaction.response.defer()
  2. Use ephemeral responses for errors: ephemeral=True
  3. Comprehensive error handling with try/except blocks
  4. User-friendly error messages with emojis
  5. Proper logging with context and stack traces
  6. Type hints on all parameters and return values
  7. Descriptive docstrings for commands and methods

Package Organization

  1. 2-4 commands per file maximum
  2. Logical grouping by functionality/domain
  3. Consistent naming patterns across packages
  4. Module-level logging for clean, consistent logs
  5. Loop-based cog loading for error resilience
  6. Comprehensive return values from setup functions

Error Handling

  1. Package-level isolation - one failed cog doesn't break the package
  2. Clear error logging with stack traces for debugging
  3. User-friendly messages that don't expose internal errors
  4. Graceful degradation when possible
  5. Metric reporting for monitoring (success/failure counts)

📊 Monitoring & Metrics

Startup Logging

The command loading system provides comprehensive metrics:

INFO - Loading players commands...
INFO - ✅ Loaded PlayerInfoCommands
INFO - 🎉 All 1 player command modules loaded successfully
INFO - ✅ players commands loaded successfully (1 cogs)
INFO - 🎉 All command packages loaded successfully (1 total cogs)

Error Scenarios

ERROR - ❌ Failed to load PlayerInfoCommands: <error details>
WARNING - ⚠️  Player commands loaded with issues: 0 successful, 1 failed
WARNING - Failed modules: PlayerInfoCommands

Command Sync Logging

INFO - Development mode: changes detected, syncing commands...
INFO - Synced 1 commands to guild 123456789

or

INFO - Development mode: no command changes detected, skipping sync

🔧 Troubleshooting

Common Issues

Import Errors

  • Check that __init__.py files exist in all packages
  • Verify cog class names match imports
  • Ensure service dependencies are available

Command Not Loading

  • Check logs for specific error messages
  • Verify cog is added to the package's cog list
  • Test individual module imports in Python REPL

Commands Not Syncing

  • Check if running in development mode (config.is_development)
  • Verify .last_command_hash file permissions
  • Use manual /sync command for troubleshooting
  • Check Discord API rate limits

Performance Issues

  • Monitor command loading times in logs
  • Check for unnecessary API calls during startup
  • Verify hash-based sync is working correctly

Debugging Tips

  1. Use the logs - comprehensive logging shows exactly what's happening
  2. Test imports individually - isolate package/module issues
  3. Check hash file - verify command change detection is working
  4. Monitor Discord API - watch for rate limiting or errors
  5. Use development mode - auto-sync helps debug command issues

📦 Command Groups Pattern

⚠️ CRITICAL: Use app_commands.Group, NOT commands.GroupCog

Discord.py provides two ways to create command groups (e.g., /injury roll, /injury clear):

  1. app_commands.Group RECOMMENDED - Use this pattern
  2. commands.GroupCog AVOID - Has interaction timing issues

Why commands.GroupCog Fails

commands.GroupCog has a critical bug that causes duplicate interaction processing, leading to:

  • 404 "Unknown interaction" errors when trying to defer/respond
  • Interaction already acknowledged errors in error handlers
  • Commands fail randomly even with proper error handling

Root Cause: GroupCog triggers the command handler twice for a single interaction, causing the first execution to consume the interaction token before the second execution can respond.

Correct Pattern: app_commands.Group

Use the same pattern as ChartCategoryGroup and ChartManageGroup:

from discord import app_commands
from discord.ext import commands
from utils.decorators import logged_command

class InjuryGroup(app_commands.Group):
    """Injury management command group."""

    def __init__(self):
        super().__init__(
            name="injury",
            description="Injury management commands"
        )
        self.logger = get_contextual_logger(f'{__name__}.InjuryGroup')

    @app_commands.command(name="roll", description="Roll for injury")
    @logged_command("/injury roll")
    async def injury_roll(self, interaction: discord.Interaction, player_name: str):
        """Roll for injury using player's injury rating."""
        await interaction.response.defer()

        # Command implementation
        # No try/catch needed - @logged_command handles it

async def setup(bot: commands.Bot):
    """Setup function for loading the injury commands."""
    bot.tree.add_command(InjuryGroup())

Key Differences

Feature app_commands.Group commands.GroupCog
Registration bot.tree.add_command(Group()) await bot.add_cog(Cog(bot))
Initialization __init__(self) no bot param __init__(self, bot) requires bot
Decorator Support @logged_command works perfectly Causes duplicate execution
Interaction Handling Single execution, reliable Duplicate execution, 404 errors
Recommended Use All command groups Never use

Migration from GroupCog to Group

If you have an existing commands.GroupCog, convert it:

  1. Change class inheritance:

    # Before
    class InjuryCog(commands.GroupCog, name="injury"):
        def __init__(self, bot):
            self.bot = bot
            super().__init__()
    
    # After
    class InjuryGroup(app_commands.Group):
        def __init__(self):
            super().__init__(name="injury", description="...")
    
  2. Update registration:

    # Before
    await bot.add_cog(InjuryCog(bot))
    
    # After
    bot.tree.add_command(InjuryGroup())
    
  3. Remove duplicate interaction checks:

    # Before (needed for GroupCog bug workaround)
    if not interaction.response.is_done():
        await interaction.response.defer()
    
    # After (clean, simple)
    await interaction.response.defer()
    

Working Examples

Good examples to reference:

  • commands/utilities/charts.py - ChartManageGroup and ChartCategoryGroup
  • commands/injuries/management.py - InjuryGroup

Both use app_commands.Group successfully with @logged_command decorators.

🚦 Future Enhancements

Planned Features

  • Permission Decorators: Role-based command restrictions per package
  • Dynamic Loading: Hot-reload commands without bot restart
  • Usage Metrics: Command usage tracking and analytics
  • Rate Limiting: Per-command rate limiting for resource management

Architecture Improvements

  • Shared Utilities: Common embed builders, decorators, helpers
  • Configuration: Per-package configuration and feature flags
  • Testing: Automated testing for command packages
  • Documentation: Auto-generated command documentation
  • Monitoring: Health checks and performance metrics per package

This architecture provides a solid foundation for scaling the Discord bot while maintaining code quality, reliability, and developer productivity.


Last Updated: Phase 2.1 - Command Package Conversion
Next Review: After implementing teams/ and league/ packages