● Terminal Client Improvement Plan - Part 2: Robust Argument Parsing Overview Replace manual string splitting with shlex for robust argument parsing that handles quoted strings, edge cases, and complex arguments properly. Files to Create 1. Create backend/terminal_client/arg_parser.py """ Argument parsing utilities for terminal client commands. Provides robust parsing for both REPL and CLI commands using shlex to handle quoted strings, spaces, and complex arguments. Author: Claude Date: 2025-10-27 """ import shlex import logging from typing import Dict, Any, List, Optional, Tuple logger = logging.getLogger(f'{__name__}.arg_parser') class ArgumentParseError(Exception): """Raised when argument parsing fails.""" pass class CommandArgumentParser: """Parse command-line style arguments for terminal client.""" @staticmethod def parse_args(arg_string: str, schema: Dict[str, Any]) -> Dict[str, Any]: """ Parse argument string according to schema. Args: arg_string: Raw argument string from command schema: Dictionary defining expected arguments { 'league': {'type': str, 'default': 'sba'}, 'home-team': {'type': int, 'default': 1}, 'count': {'type': int, 'default': 1, 'positional': True}, 'verbose': {'type': bool, 'flag': True} } Returns: Dictionary of parsed arguments with defaults applied Raises: ArgumentParseError: If parsing fails or validation fails """ try: # Use shlex for robust parsing tokens = shlex.split(arg_string) if arg_string.strip() else [] except ValueError as e: raise ArgumentParseError(f"Invalid argument syntax: {e}") # Initialize result with defaults result = {} for key, spec in schema.items(): if 'default' in spec: result[key] = spec['default'] # Track which positional arg we're on positional_keys = [k for k, v in schema.items() if v.get('positional', False)] positional_index = 0 i = 0 while i < len(tokens): token = tokens[i] # Handle flags (--option or -o) if token.startswith('--'): option_name = token[2:] # Convert hyphen to underscore for Python compatibility option_key = option_name.replace('-', '_') if option_key not in schema: raise ArgumentParseError(f"Unknown option: {token}") spec = schema[option_key] # Boolean flags don't need a value if spec.get('flag', False): result[option_key] = True i += 1 continue # Option requires a value if i + 1 >= len(tokens): raise ArgumentParseError(f"Option {token} requires a value") value_str = tokens[i + 1] # Type conversion try: if spec['type'] == int: result[option_key] = int(value_str) elif spec['type'] == float: result[option_key] = float(value_str) elif spec['type'] == list: # Parse comma-separated list result[option_key] = [item.strip() for item in value_str.split(',')] elif spec['type'] == 'int_list': # Parse comma-separated integers result[option_key] = [int(item.strip()) for item in value_str.split(',')] else: result[option_key] = value_str except ValueError as e: raise ArgumentParseError( f"Invalid value for {token}: expected {spec['type'].__name__}, got '{value_str}'" ) i += 2 # Handle positional arguments else: if positional_index >= len(positional_keys): raise ArgumentParseError(f"Unexpected positional argument: {token}") key = positional_keys[positional_index] spec = schema[key] try: if spec['type'] == int: result[key] = int(token) elif spec['type'] == float: result[key] = float(token) else: result[key] = token except ValueError as e: raise ArgumentParseError( f"Invalid value for {key}: expected {spec['type'].__name__}, got '{token}'" ) positional_index += 1 i += 1 return result @staticmethod def parse_game_id(arg_string: str) -> Optional[str]: """ Parse a game ID from argument string. Args: arg_string: Raw argument string Returns: Game ID string or None """ try: tokens = shlex.split(arg_string) if arg_string.strip() else [] # Look for --game-id option for i, token in enumerate(tokens): if token == '--game-id' and i + 1 < len(tokens): return tokens[i + 1] # If no option, check if there's a positional UUID-like argument if tokens and len(tokens[0]) == 36: # UUID length return tokens[0] return None except ValueError: return None # Predefined schemas for common commands NEW_GAME_SCHEMA = { 'league': {'type': str, 'default': 'sba'}, 'home_team': {'type': int, 'default': 1}, 'away_team': {'type': int, 'default': 2} } DEFENSIVE_SCHEMA = { 'alignment': {'type': str, 'default': 'normal'}, 'infield': {'type': str, 'default': 'normal'}, 'outfield': {'type': str, 'default': 'normal'}, 'hold': {'type': 'int_list', 'default': []} } OFFENSIVE_SCHEMA = { 'approach': {'type': str, 'default': 'normal'}, 'steal': {'type': 'int_list', 'default': []}, 'hit_run': {'type': bool, 'flag': True, 'default': False}, 'bunt': {'type': bool, 'flag': True, 'default': False} } QUICK_PLAY_SCHEMA = { 'count': {'type': int, 'default': 1, 'positional': True} } USE_GAME_SCHEMA = { 'game_id': {'type': str, 'positional': True} } def parse_new_game_args(arg_string: str) -> Dict[str, Any]: """Parse arguments for new_game command.""" return CommandArgumentParser.parse_args(arg_string, NEW_GAME_SCHEMA) def parse_defensive_args(arg_string: str) -> Dict[str, Any]: """Parse arguments for defensive command.""" return CommandArgumentParser.parse_args(arg_string, DEFENSIVE_SCHEMA) def parse_offensive_args(arg_string: str) -> Dict[str, Any]: """Parse arguments for offensive command.""" return CommandArgumentParser.parse_args(arg_string, OFFENSIVE_SCHEMA) def parse_quick_play_args(arg_string: str) -> Dict[str, Any]: """Parse arguments for quick_play command.""" return CommandArgumentParser.parse_args(arg_string, QUICK_PLAY_SCHEMA) def parse_use_game_args(arg_string: str) -> Dict[str, Any]: """Parse arguments for use_game command.""" return CommandArgumentParser.parse_args(arg_string, USE_GAME_SCHEMA) Files to Update 2. Update backend/terminal_client/repl.py # Add import at top from terminal_client.arg_parser import ( parse_new_game_args, parse_defensive_args, parse_offensive_args, parse_quick_play_args, parse_use_game_args, ArgumentParseError ) # Replace do_new_game method: def do_new_game(self, arg): """ Create a new game with lineups and start it. Usage: new_game [--league sba|pd] [--home-team N] [--away-team N] Examples: new_game new_game --league pd new_game --home-team 5 --away-team 3 """ async def _new_game(): try: # Parse arguments with robust parser args = parse_new_game_args(arg) # Use shared command gid, success = await game_commands.create_new_game( league=args['league'], home_team=args['home_team'], away_team=args['away_team'], set_current=True ) if success: self.current_game_id = gid except ArgumentParseError as e: display.print_error(f"Invalid arguments: {e}") except Exception as e: display.print_error(f"Failed to create game: {e}") logger.exception("New game error") self._run_async(_new_game()) # Replace do_defensive method: def do_defensive(self, arg): """ Submit defensive decision. Usage: defensive [--alignment TYPE] [--infield DEPTH] [--outfield DEPTH] [--hold BASES] Options: --alignment normal, shifted_left, shifted_right, extreme_shift --infield in, normal, back, double_play --outfield in, normal, back --hold Comma-separated bases (e.g., 1,3) Examples: defensive defensive --alignment shifted_left defensive --infield double_play --hold 1,3 """ async def _defensive(): try: gid = self._ensure_game() await self._ensure_game_loaded(gid) # Parse arguments args = parse_defensive_args(arg) # Submit decision await game_commands.submit_defensive_decision( game_id=gid, alignment=args['alignment'], infield=args['infield'], outfield=args['outfield'], hold_runners=args['hold'] ) except ArgumentParseError as e: display.print_error(f"Invalid arguments: {e}") except ValueError: pass # Already printed error except Exception as e: display.print_error(f"Failed: {e}") logger.exception("Defensive error") self._run_async(_defensive()) # Replace do_offensive method: def do_offensive(self, arg): """ Submit offensive decision. Usage: offensive [--approach TYPE] [--steal BASES] [--hit-run] [--bunt] Options: --approach normal, contact, power, patient --steal Comma-separated bases (e.g., 2,3) --hit-run Enable hit-and-run (flag) --bunt Attempt bunt (flag) Examples: offensive offensive --approach power offensive --steal 2 --hit-run """ async def _offensive(): try: gid = self._ensure_game() await self._ensure_game_loaded(gid) # Parse arguments args = parse_offensive_args(arg) # Submit decision await game_commands.submit_offensive_decision( game_id=gid, approach=args['approach'], steal_attempts=args['steal'], hit_and_run=args['hit_run'], bunt_attempt=args['bunt'] ) except ArgumentParseError as e: display.print_error(f"Invalid arguments: {e}") except ValueError: pass except Exception as e: display.print_error(f"Failed: {e}") logger.exception("Offensive error") self._run_async(_offensive()) # Replace do_quick_play method: def do_quick_play(self, arg): """ Auto-play multiple plays with default decisions. Usage: quick_play [COUNT] Examples: quick_play # Play 1 play quick_play 10 # Play 10 plays quick_play 27 # Play ~3 innings """ async def _quick_play(): try: gid = self._ensure_game() await self._ensure_game_loaded(gid) # Parse arguments args = parse_quick_play_args(arg) # Execute quick play plays_completed = await game_commands.quick_play_rounds( game_id=gid, count=args['count'] ) display.print_success(f"Completed {plays_completed} plays") except ArgumentParseError as e: display.print_error(f"Invalid arguments: {e}") except ValueError: pass except Exception as e: display.print_error(f"Failed: {e}") logger.exception("Quick play error") self._run_async(_quick_play()) # Replace do_use_game method: def do_use_game(self, arg): """ Switch to a different game. Usage: use_game Example: use_game a1b2c3d4-e5f6-7890-abcd-ef1234567890 """ try: # Parse arguments args = parse_use_game_args(arg) gid = UUID(args['game_id']) self.current_game_id = gid Config.set_current_game(gid) display.print_success(f"Switched to game: {gid}") except ArgumentParseError as e: display.print_error(f"Invalid arguments: {e}") except ValueError: display.print_error(f"Invalid UUID: {arg}") Testing Plan 3. Create backend/tests/unit/terminal_client/test_arg_parser.py """ Unit tests for argument parser. """ import pytest from terminal_client.arg_parser import ( CommandArgumentParser, ArgumentParseError, parse_new_game_args, parse_defensive_args, parse_offensive_args ) class TestCommandArgumentParser: """Tests for CommandArgumentParser.""" def test_parse_simple_string_arg(self): """Test parsing simple string argument.""" schema = {'name': {'type': str, 'default': 'default'}} result = CommandArgumentParser.parse_args('--name test', schema) assert result['name'] == 'test' def test_parse_integer_arg(self): """Test parsing integer argument.""" schema = {'count': {'type': int, 'default': 1}} result = CommandArgumentParser.parse_args('--count 42', schema) assert result['count'] == 42 def test_parse_flag_arg(self): """Test parsing boolean flag.""" schema = { 'verbose': {'type': bool, 'flag': True, 'default': False} } result = CommandArgumentParser.parse_args('--verbose', schema) assert result['verbose'] is True def test_parse_int_list(self): """Test parsing comma-separated integer list.""" schema = {'bases': {'type': 'int_list', 'default': []}} result = CommandArgumentParser.parse_args('--bases 1,2,3', schema) assert result['bases'] == [1, 2, 3] def test_parse_quoted_string(self): """Test parsing quoted string with spaces.""" schema = {'message': {'type': str, 'default': ''}} result = CommandArgumentParser.parse_args('--message "hello world"', schema) assert result['message'] == 'hello world' def test_parse_positional_arg(self): """Test parsing positional argument.""" schema = { 'count': {'type': int, 'positional': True, 'default': 1} } result = CommandArgumentParser.parse_args('10', schema) assert result['count'] == 10 def test_parse_mixed_args(self): """Test parsing mix of options and positional.""" schema = { 'count': {'type': int, 'positional': True, 'default': 1}, 'league': {'type': str, 'default': 'sba'} } result = CommandArgumentParser.parse_args('5 --league pd', schema) assert result['count'] == 5 assert result['league'] == 'pd' def test_parse_unknown_option_raises(self): """Test that unknown option raises error.""" schema = {'name': {'type': str, 'default': 'default'}} with pytest.raises(ArgumentParseError, match="Unknown option"): CommandArgumentParser.parse_args('--invalid test', schema) def test_parse_missing_value_raises(self): """Test that missing value for option raises error.""" schema = {'name': {'type': str, 'default': 'default'}} with pytest.raises(ArgumentParseError, match="requires a value"): CommandArgumentParser.parse_args('--name', schema) def test_parse_invalid_type_raises(self): """Test that invalid type conversion raises error.""" schema = {'count': {'type': int, 'default': 1}} with pytest.raises(ArgumentParseError, match="expected int"): CommandArgumentParser.parse_args('--count abc', schema) def test_parse_empty_string(self): """Test parsing empty string returns defaults.""" schema = { 'name': {'type': str, 'default': 'default'}, 'count': {'type': int, 'default': 1} } result = CommandArgumentParser.parse_args('', schema) assert result['name'] == 'default' assert result['count'] == 1 def test_parse_hyphen_to_underscore(self): """Test that hyphens in options convert to underscores.""" schema = {'home_team': {'type': int, 'default': 1}} result = CommandArgumentParser.parse_args('--home-team 5', schema) assert result['home_team'] == 5 class TestPrebuiltParsers: """Tests for pre-built parser functions.""" def test_parse_new_game_args_defaults(self): """Test new_game parser with defaults.""" result = parse_new_game_args('') assert result['league'] == 'sba' assert result['home_team'] == 1 assert result['away_team'] == 2 def test_parse_new_game_args_custom(self): """Test new_game parser with custom values.""" result = parse_new_game_args('--league pd --home-team 5 --away-team 3') assert result['league'] == 'pd' assert result['home_team'] == 5 assert result['away_team'] == 3 def test_parse_defensive_args_defaults(self): """Test defensive parser with defaults.""" result = parse_defensive_args('') assert result['alignment'] == 'normal' assert result['infield'] == 'normal' assert result['outfield'] == 'normal' assert result['hold'] == [] def test_parse_defensive_args_with_hold(self): """Test defensive parser with hold runners.""" result = parse_defensive_args('--alignment shifted_left --hold 1,3') assert result['alignment'] == 'shifted_left' assert result['hold'] == [1, 3] def test_parse_offensive_args_flags(self): """Test offensive parser with flags.""" result = parse_offensive_args('--approach power --hit-run --bunt') assert result['approach'] == 'power' assert result['hit_run'] is True assert result['bunt'] is True def test_parse_offensive_args_steal(self): """Test offensive parser with steal attempts.""" result = parse_offensive_args('--steal 2,3') assert result['steal'] == [2, 3] Benefits of Improved Parsing 1. Handles quoted strings: --message "double steal attempt" works correctly 2. Better error messages: Clear feedback on what went wrong 3. Type validation: Automatic conversion with helpful errors 4. Consistent behavior: Same parsing logic for REPL and CLI 5. Extensible: Easy to add new argument types 6. Edge case handling: Properly handles empty strings, trailing spaces, etc. Example Usage # Before (manual parsing, breaks on spaces): ⚾ > defensive --hold 1,3 # Works ⚾ > defensive --alignment shifted left # Breaks! (sees 'shifted' and 'left' separately) # After (shlex parsing, handles correctly): ⚾ > defensive --hold 1,3 # Still works ⚾ > defensive --alignment "shifted left" # Now works with quotes! ⚾ > quick_play 10 # Positional arg works ⚾ > offensive --hit-run --bunt # Multiple flags work