From 4baabf2fa0133902c58f69895ffa49e864bd9c5d Mon Sep 17 00:00:00 2001 From: ldastey-dev Date: Sun, 7 Jun 2026 13:01:39 +0100 Subject: [PATCH 1/5] feat: add timeout-check report and refactor CLI args Add new timeout-check subcommand that inspects in-progress team matches for actual timeouts and games where a team member's clock is running low. New features: - TimeoutCheckReport with two xlsx tabs (by-match alerts + by-player summary) plus concise stdout output - get_match_board() API method for fetching board-level game data with move_by timestamps - TimeoutAlert domain model and timeout risk analysis functions (calculate_hours_remaining, is_timeout_risk) - TIMEOUT_THRESHOLD_HOURS configurable via env var (default: 5h) CLI refactoring: - All subcommands now accept --club-ref and --club-name flags - match-participation accepts --year - prospects accepts --clubs and --exclusion-club - timeout-check accepts positional match IDs (or 'all') and --threshold - CLI args take precedence over env vars via _apply_cli_overrides() - Shared parent parser eliminates flag duplication Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .env.template | 5 +- AGENTS.md | 15 +- CLAUDE.md | 2 +- src/chesscom/api/client.py | 16 ++ src/chesscom/cli.py | 168 +++++++++-- src/chesscom/config.py | 23 +- src/chesscom/domain/models.py | 52 +++- src/chesscom/domain/services.py | 46 ++- src/chesscom/export/excel.py | 8 +- src/chesscom/reports/base.py | 4 +- src/chesscom/reports/match_eligibility.py | 30 +- src/chesscom/reports/member_summary.py | 4 +- src/chesscom/reports/timeout_check.py | 331 ++++++++++++++++++++++ tests/conftest.py | 22 +- tests/unit/test_calculations.py | 1 - tests/unit/test_cli.py | 201 +++++++++++++ tests/unit/test_config.py | 45 +++ tests/unit/test_excel.py | 4 +- tests/unit/test_models.py | 19 +- tests/unit/test_reports.py | 280 ++++++++++++++++-- tests/unit/test_services.py | 57 ++++ 21 files changed, 1216 insertions(+), 117 deletions(-) create mode 100644 src/chesscom/reports/timeout_check.py diff --git a/.env.template b/.env.template index eb3cb53..2f4a54c 100644 --- a/.env.template +++ b/.env.template @@ -3,8 +3,9 @@ # Copy this file to .env and fill in the values. # MATCH_ID=[MATCH ID] # match-eligibility (optional — can also be passed via --match-id) -CLUB_REF=[CLUB REF] # member-summary, match-participation, match-eligibility -CLUB_NAME=[CLUB NAME] # member-summary, match-participation, match-eligibility +CLUB_REF=[CLUB REF] # member-summary, match-participation, match-eligibility, timeout-check +CLUB_NAME=[CLUB NAME] # member-summary, match-participation, match-eligibility, timeout-check DATA_ANALYSIS_YEAR=[YEAR] # match-participation LIST_OF_CLUBS=[COMMA SEPARATED LIST OF CLUBS] # prospects EXCLUSION_CLUB=[CLUB REF] # prospects (optional — members of this club are excluded from the prospect list) +TIMEOUT_THRESHOLD_HOURS=[HOURS] # timeout-check (optional — hours remaining below which a game is flagged; default: 5) diff --git a/AGENTS.md b/AGENTS.md index 092fe67..d50e1af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,11 +7,12 @@ ## Project Overview A Python CLI tool for Chess.com club administrators. It fetches data from the -Chess.com public API and generates Excel workbooks covering four report types: +Chess.com public API and generates Excel workbooks covering five report types: club member summaries, match participation analysis, prospect identification, -and match eligibility checking. Club admins use it to manage rosters, track -member contributions, and identify new member candidates — delivering the kind -of structured reporting that the Chess.com UI does not provide out of the box. +match eligibility checking, and timeout monitoring. Club admins use it to +manage rosters, track member contributions, identify new member candidates, +and monitor in-progress matches for timeout risk — delivering the kind of +structured reporting that the Chess.com UI does not provide out of the box. --- @@ -144,7 +145,8 @@ chesscom/ │ ├── match_eligibility.py │ ├── match_participation.py │ ├── member_summary.py -│ └── prospect.py +│ ├── prospect.py +│ └── timeout_check.py ├── tests/ │ ├── conftest.py │ ├── integration/ # Live-API tests (require network access) @@ -183,7 +185,8 @@ chesscom/ subcommand handler in `cli.py`. - **Adding a config variable:** Add the field to `AppConfig` in `config.py` and - parse it in `from_env()`. Never read env vars anywhere else. + parse it in `from_env()`. Never read env vars anywhere else. Add a + corresponding CLI flag in `cli.py` and handle it in `_apply_cli_overrides()`. - **Adding an API endpoint:** Add a method to `ChessComClient` in `api/client.py`. Route all HTTP calls through `_get()`. diff --git a/CLAUDE.md b/CLAUDE.md index 7236b76..db58efa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,6 +18,6 @@ Assessment, review, planning, and refactoring playbooks are available as Claude - **Plan mode for non-trivial tasks** — if a task has 3+ steps or architectural impact, write a plan to `tasks/todo.md` before implementing. See `.context/conventions/workflow.md`. - **No type-checker or security-audit tool** — `mypy` and `bandit` are not configured. Do not assume they exist or add them without discussion. - **Chess.com API domain knowledge** — the public API (`api.chess.com/pub`) is unauthenticated. All endpoint methods live in `ChessComClient`. The Chrome `User-Agent` header is required and must not be removed. -- **Four report types only** — `member-summary`, `match-participation`, `prospects`, `match-eligibility`. New report types must follow the `BaseReport` subclass pattern and be registered in `cli.py`. +- **Five report types** — `member-summary`, `match-participation`, `prospects`, `match-eligibility`, `timeout-check`. New report types must follow the `BaseReport` subclass pattern and be registered in `cli.py`. - **Environment variables drive all configuration** — every setting is in `.env.template`. New settings must be added to `AppConfig` in `config.py` first; never use `os.getenv()` directly. - **Verify tests pass before marking work complete** — run `pytest` after any behaviour change. diff --git a/src/chesscom/api/client.py b/src/chesscom/api/client.py index a78d3ae..de08715 100644 --- a/src/chesscom/api/client.py +++ b/src/chesscom/api/client.py @@ -184,3 +184,19 @@ def get_match(self, match_id_or_url: str) -> dict: return self._get( match_id_or_url if match_id_or_url.startswith("http") else f"match/{match_id_or_url}" ) + + def get_match_board(self, board_url: str) -> dict: + """Return board detail for a team match board. + + The board endpoint returns the individual games on a specific board, + including ``move_by`` timestamps for in-progress daily games. + + Args: + board_url: Fully-qualified board URL as returned in the player's + ``board`` field from a match detail response (e.g. + ``"https://api.chess.com/pub/match/12345/1"``). + + Returns: + Board detail dict containing ``board_scores`` and ``games``. + """ + return self._get(board_url) diff --git a/src/chesscom/cli.py b/src/chesscom/cli.py index 27c375c..0a9e1a2 100644 --- a/src/chesscom/cli.py +++ b/src/chesscom/cli.py @@ -1,17 +1,19 @@ """Command-line interface for the Chess.com club management tools. -Exposes four subcommands, each corresponding to one of the report classes: +Exposes five subcommands, each corresponding to one of the report classes: * ``match-participation`` — :class:`~chesscom.reports.match_participation.MatchParticipationReport` * ``member-summary`` — :class:`~chesscom.reports.member_summary.MemberSummaryReport` * ``prospects`` — :class:`~chesscom.reports.prospect.ProspectReport` * ``match-eligibility`` — :class:`~chesscom.reports.match_eligibility.MatchEligibilityReport` +* ``timeout-check`` — :class:`~chesscom.reports.timeout_check.TimeoutCheckReport` Usage:: python -m chesscom [options] All configuration is loaded from environment variables (see ``.env.template``). +CLI arguments override environment variables when both are provided. A ``.env`` file in the project root is automatically sourced at startup. """ @@ -31,6 +33,45 @@ from chesscom.reports.match_participation import MatchParticipationReport from chesscom.reports.member_summary import MemberSummaryReport from chesscom.reports.prospect import ProspectReport +from chesscom.reports.timeout_check import TimeoutCheckReport + +# --------------------------------------------------------------------------- +# CLI → config merging +# --------------------------------------------------------------------------- + + +def _apply_cli_overrides(config: AppConfig, args: argparse.Namespace) -> AppConfig: + """Return a copy of *config* with fields overridden by CLI arguments. + + Only non-``None`` CLI values are applied; absent flags leave the + environment-sourced config value unchanged. + + Args: + config: Base configuration loaded from environment variables. + args: Parsed CLI arguments. + + Returns: + A new :class:`AppConfig` instance with overrides applied. + """ + overrides: dict = {} + + if getattr(args, "club_ref", None): + overrides["club_ref"] = args.club_ref + if getattr(args, "club_name", None): + overrides["club_name"] = args.club_name + if getattr(args, "match_id", None): + overrides["match_id"] = args.match_id + if getattr(args, "year", None) is not None: + overrides["data_analysis_year"] = args.year + if getattr(args, "clubs", None): + overrides["prospect_clubs"] = args.clubs + if getattr(args, "exclusion_club", None): + overrides["exclusion_club"] = args.exclusion_club + if getattr(args, "threshold", None) is not None: + overrides["timeout_threshold_hours"] = args.threshold + + return dataclasses.replace(config, **overrides) if overrides else config + # --------------------------------------------------------------------------- # Timing helper @@ -51,29 +92,37 @@ def _run_timed(report: BaseReport) -> None: # --------------------------------------------------------------------------- -def _handle_match_participation(args: argparse.Namespace) -> None: # noqa: ARG001 - config = AppConfig.from_env() +def _handle_match_participation(args: argparse.Namespace) -> None: + config = _apply_cli_overrides(AppConfig.from_env(), args) _run_timed(MatchParticipationReport(ChessComClient(), config)) -def _handle_member_summary(args: argparse.Namespace) -> None: # noqa: ARG001 - config = AppConfig.from_env() +def _handle_member_summary(args: argparse.Namespace) -> None: + config = _apply_cli_overrides(AppConfig.from_env(), args) _run_timed(MemberSummaryReport(ChessComClient(), config)) -def _handle_prospects(args: argparse.Namespace) -> None: # noqa: ARG001 - config = AppConfig.from_env() +def _handle_prospects(args: argparse.Namespace) -> None: + config = _apply_cli_overrides(AppConfig.from_env(), args) _run_timed(ProspectReport(ChessComClient(), config)) def _handle_match_eligibility(args: argparse.Namespace) -> None: - config = AppConfig.from_env() - # --match-id CLI flag overrides (or supplies) the MATCH_ID env var - if args.match_id: - config = dataclasses.replace(config, match_id=args.match_id) + config = _apply_cli_overrides(AppConfig.from_env(), args) _run_timed(MatchEligibilityReport(ChessComClient(), config)) +def _handle_timeout_check(args: argparse.Namespace) -> None: + config = _apply_cli_overrides(AppConfig.from_env(), args) + report = TimeoutCheckReport(ChessComClient(), config, match_ids=args.match_ids) + start = time.monotonic() + path = report.run() + elapsed = time.monotonic() - start + print(report.format_console_summary()) + print(f"Report written: {path}") + print(f"Execution time: {elapsed:.2f}s ({elapsed / 60:.2f} min)") + + # --------------------------------------------------------------------------- # Argument parser # --------------------------------------------------------------------------- @@ -83,6 +132,7 @@ def _handle_match_eligibility(args: argparse.Namespace) -> None: "member-summary": _handle_member_summary, "prospects": _handle_prospects, "match-eligibility": _handle_match_eligibility, + "timeout-check": _handle_timeout_check, } @@ -95,19 +145,43 @@ def error(self, message: str) -> None: # noqa: D102 sys.exit(2) +def _common_parser() -> argparse.ArgumentParser: + """Return a parent parser with flags shared by every subcommand. + + These flags override the corresponding environment variables when + provided. + """ + parent = argparse.ArgumentParser(add_help=False) + parent.add_argument( + "--club-ref", + metavar="SLUG", + default=None, + help="Club slug for API paths (overrides CLUB_REF env var).", + ) + parent.add_argument( + "--club-name", + metavar="NAME", + default=None, + help="Club display name (overrides CLUB_NAME env var).", + ) + return parent + + def build_parser() -> argparse.ArgumentParser: """Construct and return the top-level :class:`argparse.ArgumentParser`. Returns: - Fully configured parser with all four subcommands registered. + Fully configured parser with all five subcommands registered. """ + common = _common_parser() + parser = _HelpOnErrorParser( prog="chesscom", description="Chess.com club management tools.", formatter_class=argparse.RawDescriptionHelpFormatter, epilog=( - "All options are read from environment variables.\n" - "Copy .env.template to .env and fill in the values before running." + "Configuration is read from environment variables (see .env.template).\n" + "CLI arguments override environment variables when both are provided." ), ) @@ -117,17 +191,28 @@ def build_parser() -> argparse.ArgumentParser: metavar="", ) - sub.add_parser( + # -- match-participation ------------------------------------------------ + mp_parser = sub.add_parser( "match-participation", + parents=[common], help="Export club contribution / match-participation report.", description=( "Analyses member participation and win rates across all team matches " "completed in DATA_ANALYSIS_YEAR and writes a two-sheet Excel workbook." ), ) + mp_parser.add_argument( + "--year", + metavar="YYYY", + type=int, + default=None, + help="Analysis year (overrides DATA_ANALYSIS_YEAR env var).", + ) + # -- member-summary ----------------------------------------------------- sub.add_parser( "member-summary", + parents=[common], help="Export a roster of all current club members with key stats.", description=( "Fetches every club member's profile and stats from the Chess.com API " @@ -135,17 +220,37 @@ def build_parser() -> argparse.ArgumentParser: ), ) - sub.add_parser( + # -- prospects ---------------------------------------------------------- + pr_parser = sub.add_parser( "prospects", + parents=[common], help="Export a de-duplicated prospect list from multiple clubs.", description=( "Collects members from LIST_OF_CLUBS, removes anyone already in " "EXCLUSION_CLUB, de-duplicates, and exports to Excel." ), ) + pr_parser.add_argument( + "--clubs", + nargs="+", + metavar="SLUG", + default=None, + help="Club slugs to inspect for prospects (overrides LIST_OF_CLUBS env var).", + ) + pr_parser.add_argument( + "--exclusion-club", + metavar="SLUG", + default=None, + help=( + "Club whose members are excluded from the prospect list " + "(overrides EXCLUSION_CLUB env var)." + ), + ) + # -- match-eligibility -------------------------------------------------- me_parser = sub.add_parser( "match-eligibility", + parents=[common], help="Export eligible members for a specific team match.", description=( "Lists club members whose rating falls within the match cap for the " @@ -162,6 +267,37 @@ def build_parser() -> argparse.ArgumentParser: ), ) + # -- timeout-check ------------------------------------------------------ + tc_parser = sub.add_parser( + "timeout-check", + parents=[common], + help="Check matches for timeouts and near-timeout conditions.", + description=( + "Inspects in-progress team matches for actual timeouts and games " + "where a team member's clock is running low. Outputs both a " + "console summary and an Excel workbook." + ), + ) + tc_parser.add_argument( + "match_ids", + nargs="+", + metavar="MATCH_ID", + help=( + 'One or more match IDs to check, or "all" to check every ' + "in-progress match for the configured club." + ), + ) + tc_parser.add_argument( + "--threshold", + metavar="HOURS", + type=float, + default=None, + help=( + "Hours remaining below which a game is flagged as at risk " + "(overrides TIMEOUT_THRESHOLD_HOURS env var; default: 5)." + ), + ) + return parser diff --git a/src/chesscom/config.py b/src/chesscom/config.py index 1713630..a4c2913 100644 --- a/src/chesscom/config.py +++ b/src/chesscom/config.py @@ -21,6 +21,10 @@ EXCLUSION_CLUB Club slug whose members are excluded from the prospect report (defaults to ``None``; previously hard-coded as ``"team-scotland"``). + TIMEOUT_THRESHOLD_HOURS + Number of hours remaining below which an in-progress + game is flagged as at risk of timing out. Parsed to + ``float``; defaults to ``5.0`` if blank. """ from __future__ import annotations @@ -45,6 +49,9 @@ class AppConfig: prospect_clubs: Ordered list of club slugs to inspect for prospects. exclusion_club: Optional club slug whose members are excluded from the prospect report. + timeout_threshold_hours: Number of hours remaining on the clock + below which a game is flagged as at risk of timing out. + Defaults to ``5.0``. """ club_ref: str @@ -53,6 +60,7 @@ class AppConfig: match_id: str | None = field(default=None) prospect_clubs: list[str] = field(default_factory=list) exclusion_club: str | None = field(default=None) + timeout_threshold_hours: float = field(default=5.0) # ------------------------------------------------------------------ # Factory @@ -80,9 +88,7 @@ def from_env(cls) -> AppConfig: missing.append("CLUB_NAME") if missing: - raise ValueError( - f"Missing required environment variable(s): {', '.join(missing)}" - ) + raise ValueError(f"Missing required environment variable(s): {', '.join(missing)}") # --- optional fields ------------------------------------------------ data_analysis_year: int | None = None @@ -102,6 +108,16 @@ def from_env(cls) -> AppConfig: exclusion_club = os.getenv("EXCLUSION_CLUB", "").strip() or None + timeout_threshold_hours = 5.0 + raw_threshold = os.getenv("TIMEOUT_THRESHOLD_HOURS", "").strip() + if raw_threshold: + try: + timeout_threshold_hours = float(raw_threshold) + except ValueError as exc: + raise ValueError( + f"TIMEOUT_THRESHOLD_HOURS must be a number; got '{raw_threshold}'" + ) from exc + return cls( club_ref=club_ref, club_name=club_name, @@ -109,4 +125,5 @@ def from_env(cls) -> AppConfig: match_id=match_id, prospect_clubs=prospect_clubs, exclusion_club=exclusion_club, + timeout_threshold_hours=timeout_threshold_hours, ) diff --git a/src/chesscom/domain/models.py b/src/chesscom/domain/models.py index 11ea10a..1067cc3 100644 --- a/src/chesscom/domain/models.py +++ b/src/chesscom/domain/models.py @@ -69,13 +69,9 @@ def from_api_response( chess960_daily = stats.get("chess960_daily") or {} chess960_last = chess960_daily.get("last") or {} chess960_rating_raw = chess960_last.get("rating") - chess960_rating = ( - chess960_rating_raw if isinstance(chess960_rating_raw, int) else None - ) + chess960_rating = chess960_rating_raw if isinstance(chess960_rating_raw, int) else None - timeout_percent = float( - (chess_daily.get("record") or {}).get("timeout_percent", 0.0) - ) + timeout_percent = float((chess_daily.get("record") or {}).get("timeout_percent", 0.0)) joined_club: datetime | None = ( datetime.fromtimestamp(joined_club_timestamp, tz=UTC) @@ -90,12 +86,8 @@ def from_api_response( daily_rating=daily_rating, chess960_rating=chess960_rating, timeout_percent=timeout_percent, - joined_chess_com=datetime.fromtimestamp( - profile.get("joined", 0), tz=UTC - ), - last_online=datetime.fromtimestamp( - profile.get("last_online", 0), tz=UTC - ), + joined_chess_com=datetime.fromtimestamp(profile.get("joined", 0), tz=UTC), + last_online=datetime.fromtimestamp(profile.get("last_online", 0), tz=UTC), joined_club=joined_club, ) @@ -200,9 +192,7 @@ def from_api_response( match_id=match_id, name=data.get("name", ""), url=url, - start_time=datetime.fromtimestamp( - data.get("start_time", 0), tz=UTC - ), + start_time=datetime.fromtimestamp(data.get("start_time", 0), tz=UTC), max_rating=max_rating, variant="chess960" if is_chess960 else "chess", participants=participants, @@ -240,3 +230,35 @@ class MemberParticipation: timeouts: int participation_pct: float win_rate_pct: float + + +@dataclass +class TimeoutAlert: + """Records a timeout or near-timeout condition for a player in a match. + + Emitted by the timeout-check report when a team member has either + already timed out or has an in-progress game where ``move_by`` is + within the configured threshold. + + Attributes: + match_name: Human-readable match name. + match_id: Numeric match identifier as a string. + username: Chess.com username of the affected player. + board_url: Fully-qualified API URL for the board. + colour: ``"white"`` or ``"black"`` — the side the player was on. + status: ``"timed_out"`` (game already lost on time) or + ``"at_risk"`` (clock running low). + move_by: UTC datetime when the next move must be made, or ``None`` + for completed timeouts. + hours_remaining: Hours until ``move_by``, or ``None`` for completed + timeouts. + """ + + match_name: str + match_id: str + username: str + board_url: str + colour: str + status: str + move_by: datetime | None + hours_remaining: float | None diff --git a/src/chesscom/domain/services.py b/src/chesscom/domain/services.py index e867e62..0b6bf24 100644 --- a/src/chesscom/domain/services.py +++ b/src/chesscom/domain/services.py @@ -7,6 +7,7 @@ from __future__ import annotations +from datetime import UTC, datetime from typing import Literal from chesscom.domain.models import Match, MatchResult, Member, MemberParticipation @@ -214,11 +215,7 @@ def build_participation_stats( for match in matches: player_result: MatchResult | None = next( - ( - p - for p in match.participants - if p.username.lower() == username_lower - ), + (p for p in match.participants if p.username.lower() == username_lower), None, ) if player_result is None: @@ -238,9 +235,7 @@ def build_participation_stats( timeouts += 1 losses += 1 # timeouts count as losses for win-rate - participation_pct = calculate_participation_percentage( - total_matches, matches_participated - ) + participation_pct = calculate_participation_percentage(total_matches, matches_participated) win_rate_pct = calculate_win_rate(wins, losses, draws) return MemberParticipation( @@ -255,3 +250,38 @@ def build_participation_stats( participation_pct=participation_pct, win_rate_pct=win_rate_pct, ) + + +# --------------------------------------------------------------------------- +# Timeout risk analysis +# --------------------------------------------------------------------------- + + +def calculate_hours_remaining(move_by: datetime) -> float: + """Return the number of hours between now and *move_by*. + + A negative value means the deadline has already passed. + + Args: + move_by: UTC datetime when the next move must be made. + + Returns: + Hours remaining as a float, rounded to two decimal places. + """ + delta = move_by - datetime.now(tz=UTC) + return round(delta.total_seconds() / 3600, 2) + + +def is_timeout_risk(move_by: datetime, threshold_hours: float) -> bool: + """Return ``True`` when *move_by* is within *threshold_hours* of now. + + A ``move_by`` that has already passed is always considered at risk. + + Args: + move_by: UTC datetime when the next move must be made. + threshold_hours: Maximum acceptable hours remaining. + + Returns: + ``True`` if the remaining time is at or below the threshold. + """ + return calculate_hours_remaining(move_by) <= threshold_hours diff --git a/src/chesscom/export/excel.py b/src/chesscom/export/excel.py index 2dedc64..732a348 100644 --- a/src/chesscom/export/excel.py +++ b/src/chesscom/export/excel.py @@ -135,15 +135,11 @@ def _unique_path(self) -> str: candidate = os.path.join(self.output_dir, f"{self.base_name}.xlsx") counter = 1 while os.path.exists(candidate): - candidate = os.path.join( - self.output_dir, f"{self.base_name}_{counter}.xlsx" - ) + candidate = os.path.join(self.output_dir, f"{self.base_name}_{counter}.xlsx") counter += 1 return candidate - def _apply_hyperlinks( - self, writer: pd.ExcelWriter, sheet: SheetConfig - ) -> None: + def _apply_hyperlinks(self, writer: pd.ExcelWriter, sheet: SheetConfig) -> None: """Inject hyperlinks into *sheet.hyperlink_column* cells. Row 1 is the header; data rows start at row 2. diff --git a/src/chesscom/reports/base.py b/src/chesscom/reports/base.py index 006450b..8a8cb74 100644 --- a/src/chesscom/reports/base.py +++ b/src/chesscom/reports/base.py @@ -94,9 +94,7 @@ def build_sheet_configs(self, data: list[dict]) -> list[SheetConfig]: name=sheet_name, dataframe=df, hyperlink_column=hyperlink_col, - hyperlink_url_template=( - _CHESSCOM_PROFILE_URL if hyperlink_col else None - ), + hyperlink_url_template=(_CHESSCOM_PROFILE_URL if hyperlink_col else None), ) ] diff --git a/src/chesscom/reports/match_eligibility.py b/src/chesscom/reports/match_eligibility.py index 8b75293..2ee7f41 100644 --- a/src/chesscom/reports/match_eligibility.py +++ b/src/chesscom/reports/match_eligibility.py @@ -60,7 +60,9 @@ def collect_data(self) -> list[dict]: # --- Match metadata ------------------------------------------------- match_data = self.client.get_match(match_id) match = Match.from_api_response( - match_data, match_id, f"https://api.chess.com/pub/match/{match_id}", + match_data, + match_id, + f"https://api.chess.com/pub/match/{match_id}", self.config.club_name, ) variant = match.variant @@ -70,9 +72,7 @@ def collect_data(self) -> list[dict]: self._variant = variant # --- Participants already signed up --------------------------------- - signed_up_lower: set[str] = { - p.username.lower() for p in match.participants - } + signed_up_lower: set[str] = {p.username.lower() for p in match.participants} # --- All club members with full Member objects ---------------------- raw_members = self._all_club_members() @@ -92,18 +92,18 @@ def collect_data(self) -> list[dict]: # --- Build result rows ---------------------------------------------- results: list[dict] = [] for m in members: - chess960_display = ( - m.chess960_rating if m.chess960_rating is not None else "Unrated" + chess960_display = m.chess960_rating if m.chess960_rating is not None else "Unrated" + results.append( + { + "Username": m.username, + "Daily Rating": m.daily_rating if m.daily_rating is not None else "Unrated", + "Chess960 Rating": chess960_display, + "Variant": variant.upper(), + "Last Online": _fmt(m.last_online), + "Timeout Percentage": m.timeout_percent, + "Signed Up": "Yes" if m.username.lower() in signed_up_lower else "No", + } ) - results.append({ - "Username": m.username, - "Daily Rating": m.daily_rating if m.daily_rating is not None else "Unrated", - "Chess960 Rating": chess960_display, - "Variant": variant.upper(), - "Last Online": _fmt(m.last_online), - "Timeout Percentage": m.timeout_percent, - "Signed Up": "Yes" if m.username.lower() in signed_up_lower else "No", - }) return results diff --git a/src/chesscom/reports/member_summary.py b/src/chesscom/reports/member_summary.py index b0212b9..3032e1a 100644 --- a/src/chesscom/reports/member_summary.py +++ b/src/chesscom/reports/member_summary.py @@ -65,9 +65,7 @@ def _to_row(m: Member) -> dict: "Joined Club": _fmt(m.joined_club), "Last Online": _fmt(m.last_online), "Daily Rating": m.daily_rating if m.daily_rating is not None else "Unrated", - "Chess960 Rating": ( - m.chess960_rating if m.chess960_rating is not None else "Unrated" - ), + "Chess960 Rating": (m.chess960_rating if m.chess960_rating is not None else "Unrated"), "Timeout Percentage": m.timeout_percent, } diff --git a/src/chesscom/reports/timeout_check.py b/src/chesscom/reports/timeout_check.py new file mode 100644 index 0000000..4b96025 --- /dev/null +++ b/src/chesscom/reports/timeout_check.py @@ -0,0 +1,331 @@ +"""Timeout Check Report. + +Inspects in-progress team matches for actual timeouts (completed games +lost on time) and potential timeouts (in-progress games where a team +member's clock is running low). + +Outputs both an Excel workbook and a concise console summary. +""" + +from __future__ import annotations + +from collections import Counter +from datetime import UTC, datetime + +import pandas as pd + +from chesscom.domain.models import TimeoutAlert +from chesscom.domain.services import calculate_hours_remaining, is_timeout_risk +from chesscom.export.excel import SheetConfig +from chesscom.reports.base import BaseReport + +_CHESSCOM_PROFILE_URL = "https://www.chess.com/member/{value}" +_CHESSCOM_MATCH_URL = "https://www.chess.com/club/matches/{value}" + + +class TimeoutCheckReport(BaseReport): + """Checks matches for timeouts and near-timeout conditions. + + Produces a two-sheet workbook: + + * **Timeout Alerts by Match** — one row per flagged game, grouped by + match, showing the player, colour, status, and time remaining. + * **Timeout Summary by Player** — one row per player who has at least + one timeout, ordered by total timeout count descending. + + The report also provides :meth:`format_console_summary` for a concise + stdout overview. + + Args: + match_ids: List of match ID strings, or ``["all"]`` to check + every in-progress match for the configured club. + """ + + def __init__(self, client, config, *, match_ids: list[str]) -> None: + super().__init__(client, config) + self._match_ids = match_ids + self._alerts: list[TimeoutAlert] = [] + + def get_report_name(self) -> str: + return "Timeout Check Report" + + # ------------------------------------------------------------------ + # Data collection + # ------------------------------------------------------------------ + + def collect_data(self) -> list[dict]: + """Fetch match and board data, identify timeouts and at-risk games. + + Side-effect: populates ``self._alerts`` for use by + :meth:`format_console_summary` and :meth:`build_sheet_configs`. + + Returns: + List of row dicts for the by-match sheet. + """ + raw_matches = self._resolve_matches() + alerts: list[TimeoutAlert] = [] + + for raw in raw_matches: + url: str = raw.get("@id", "") + match_id = url.rstrip("/").rsplit("/", maxsplit=1)[-1] + data = self.client.get_match(url) + match_name = data.get("name", "") + + club_team = self._find_club_team(data) + if club_team is None: + continue + + players = club_team.get("players") or [] + boards_checked: set[str] = set() + + for player in players: + username = player.get("username", "") + board_url = player.get("board", "") + if not username: + continue + + # Check for completed timeouts + for colour, key in [("white", "played_as_white"), ("black", "played_as_black")]: + result = player.get(key, "") + if result == "timeout": + alerts.append( + TimeoutAlert( + match_name=match_name, + match_id=match_id, + username=username, + board_url=board_url, + colour=colour, + status="timed_out", + move_by=None, + hours_remaining=None, + ) + ) + + # Check for in-progress games (result absent or empty) + has_in_progress = any( + not player.get(key) for key in ("played_as_white", "played_as_black") + ) + + if has_in_progress and board_url and board_url not in boards_checked: + boards_checked.add(board_url) + self._check_board_for_risks(board_url, match_name, match_id, alerts) + + self._alerts = alerts + return self._alerts_to_rows(alerts) + + # ------------------------------------------------------------------ + # Sheet configuration + # ------------------------------------------------------------------ + + def build_sheet_configs(self, data: list[dict]) -> list[SheetConfig]: + alerts = self._alerts + + # Sheet 1: by-match alerts + by_match_df = ( + pd.DataFrame(data) + if data + else pd.DataFrame( + columns=[ + "Match Name", + "Match ID", + "Username", + "Board", + "Colour", + "Status", + "Move By", + "Hours Remaining", + ] + ) + ) + + # Sheet 2: by-player summary (only players with timeouts > 0) + timeout_counts: Counter[str] = Counter() + matches_with_timeouts: dict[str, set[str]] = {} + for alert in alerts: + if alert.status == "timed_out": + timeout_counts[alert.username] += 1 + matches_with_timeouts.setdefault(alert.username, set()).add(alert.match_id) + + player_rows: list[dict] = [] + for username, count in timeout_counts.most_common(): + player_rows.append( + { + "Username": username, + "Total Timeouts": count, + "Matches With Timeouts": len(matches_with_timeouts.get(username, set())), + } + ) + + by_player_df = ( + pd.DataFrame(player_rows) + if player_rows + else pd.DataFrame(columns=["Username", "Total Timeouts", "Matches With Timeouts"]) + ) + + sheets = [ + SheetConfig( + name="Timeout Alerts by Match", + dataframe=by_match_df, + hyperlink_column="Username" if "Username" in by_match_df.columns else None, + hyperlink_url_template=( + _CHESSCOM_PROFILE_URL if "Username" in by_match_df.columns else None + ), + ), + SheetConfig( + name="Timeout Summary by Player", + dataframe=by_player_df, + hyperlink_column="Username" if "Username" in by_player_df.columns else None, + hyperlink_url_template=( + _CHESSCOM_PROFILE_URL if "Username" in by_player_df.columns else None + ), + ), + ] + return sheets + + # ------------------------------------------------------------------ + # Console summary + # ------------------------------------------------------------------ + + def format_console_summary(self) -> str: + """Return a concise console summary of timeout alerts. + + Call this after :meth:`run` (which invokes :meth:`collect_data` + internally) so that ``self._alerts`` is populated. + + Returns: + Formatted multi-line string suitable for printing to stdout. + """ + if not self._alerts: + return "No timeout issues found across checked matches." + + # Group alerts by match + by_match: dict[str, list[TimeoutAlert]] = {} + for alert in self._alerts: + key = f"{alert.match_name} (ID: {alert.match_id})" + by_match.setdefault(key, []).append(alert) + + lines: list[str] = ["=== Timeout Check Summary ===", ""] + for match_label, match_alerts in by_match.items(): + timed_out = [a for a in match_alerts if a.status == "timed_out"] + at_risk = [a for a in match_alerts if a.status == "at_risk"] + lines.append(f"Match: {match_label}") + + parts: list[str] = [] + if at_risk: + parts.append(f"{len(at_risk)} at risk") + if timed_out: + parts.append(f"{len(timed_out)} timed out") + lines.append(f" {', '.join(parts)}") + + for a in timed_out: + lines.append(f" TIMED OUT: {a.username} ({a.colour})") + for a in at_risk: + hrs = f"{a.hours_remaining:.1f}" if a.hours_remaining is not None else "?" + lines.append(f" AT RISK: {a.username} ({a.colour}) — {hrs} hours remaining") + lines.append("") + + return "\n".join(lines) + + # ------------------------------------------------------------------ + # Private helpers + # ------------------------------------------------------------------ + + def _resolve_matches(self) -> list[dict]: + """Return the list of raw match summary dicts to check.""" + if self._match_ids == ["all"]: + resp = self.client.get_club_matches(self.config.club_ref) + return resp.get("in_progress", []) + + return [{"@id": f"https://api.chess.com/pub/match/{mid}"} for mid in self._match_ids] + + def _find_club_team(self, match_data: dict) -> dict | None: + """Identify the club's team (team1 or team2) within match data.""" + teams = match_data.get("teams") or {} + for team_key in ("team1", "team2"): + team = teams.get(team_key) or {} + if team.get("name") == self.config.club_name: + return team + return None + + def _check_board_for_risks( + self, + board_url: str, + match_name: str, + match_id: str, + alerts: list[TimeoutAlert], + ) -> None: + """Fetch board data and append at-risk alerts for our team's players.""" + try: + board_data = self.client.get_match_board(board_url) + except Exception: + return + + club_ref_lower = self.config.club_ref.lower() + + for game in board_data.get("games") or []: + # Skip finished games (they have an end_time) + if game.get("end_time"): + continue + + move_by_ts = game.get("move_by") + if not move_by_ts or move_by_ts == 0: + # move_by == 0 means the player-to-move is on vacation + continue + + move_by_dt = datetime.fromtimestamp(move_by_ts, tz=UTC) + if not is_timeout_risk(move_by_dt, self.config.timeout_threshold_hours): + continue + + hours_left = calculate_hours_remaining(move_by_dt) + + # Determine which side's move it is + turn = game.get("turn", "") + for colour in ("white", "black"): + if colour != turn: + continue + player_data = game.get(colour) or {} + # Check if this player is on our team + team_url = player_data.get("team", "") + player_username = player_data.get("username", "") + + if self._is_our_player(team_url, club_ref_lower): + alerts.append( + TimeoutAlert( + match_name=match_name, + match_id=match_id, + username=player_username, + board_url=board_url, + colour=colour, + status="at_risk", + move_by=move_by_dt, + hours_remaining=hours_left, + ) + ) + + @staticmethod + def _is_our_player(team_url: str, club_ref_lower: str) -> bool: + """Check if a player belongs to our club using the team URL.""" + if team_url: + return club_ref_lower in team_url.lower() + return False + + @staticmethod + def _alerts_to_rows(alerts: list[TimeoutAlert]) -> list[dict]: + """Convert alert objects to row dicts for the by-match sheet.""" + rows: list[dict] = [] + for a in alerts: + rows.append( + { + "Match Name": a.match_name, + "Match ID": a.match_id, + "Username": a.username, + "Board": a.board_url, + "Colour": a.colour, + "Status": "Timed Out" if a.status == "timed_out" else "At Risk", + "Move By": (a.move_by.strftime("%d/%m/%Y %H:%M UTC") if a.move_by else ""), + "Hours Remaining": ( + round(a.hours_remaining, 1) if a.hours_remaining is not None else "" + ), + } + ) + return rows diff --git a/tests/conftest.py b/tests/conftest.py index 0a929ce..990fc21 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -101,8 +101,16 @@ def match_detail_response_team1(): "team1": { "name": "Test Club", "players": [ - {"username": "Alice", "played_as_white": "win", "played_as_black": "checkmated"}, - {"username": "Bob", "played_as_white": "in progress", "played_as_black": "timeout"}, + { + "username": "Alice", + "played_as_white": "win", + "played_as_black": "checkmated", + }, + { + "username": "Bob", + "played_as_white": "in progress", + "played_as_black": "timeout", + }, ], }, "team2": { @@ -131,7 +139,11 @@ def match_detail_response_team2(): "team2": { "name": "Test Club", "players": [ - {"username": "Alice", "played_as_white": "win", "played_as_black": "checkmated"}, + { + "username": "Alice", + "played_as_white": "win", + "played_as_black": "checkmated", + }, ], }, }, @@ -147,7 +159,9 @@ def match_detail_response_chess960(): "teams": { "team1": { "name": "Test Club", - "players": [{"username": "Alice", "played_as_white": "win", "played_as_black": "win"}], + "players": [ + {"username": "Alice", "played_as_white": "win", "played_as_black": "win"} + ], }, "team2": { "name": "Opponent Club", diff --git a/tests/unit/test_calculations.py b/tests/unit/test_calculations.py index 1c48faf..534a858 100644 --- a/tests/unit/test_calculations.py +++ b/tests/unit/test_calculations.py @@ -5,7 +5,6 @@ and utils. Now they test the canonical implementations in the services module. """ - from chesscom.domain.services import calculate_participation_percentage, calculate_win_rate # --------------------------------------------------------------------------- diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 14ea23e..0912cfd 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -275,3 +275,204 @@ def test_execution_time_printed_on_success(self, capsys): main(["member-summary"]) captured = capsys.readouterr() assert "Execution time" in captured.out + + +# =========================================================================== +# build_parser — timeout-check subcommand +# =========================================================================== + + +class TestTimeoutCheckParser: + def test_timeout_check_subcommand(self): + args = build_parser().parse_args(["timeout-check", "12345"]) + assert args.subcommand == "timeout-check" + assert args.match_ids == ["12345"] + + def test_timeout_check_multiple_ids(self): + args = build_parser().parse_args(["timeout-check", "111", "222", "333"]) + assert args.match_ids == ["111", "222", "333"] + + def test_timeout_check_all_keyword(self): + args = build_parser().parse_args(["timeout-check", "all"]) + assert args.match_ids == ["all"] + + def test_timeout_check_threshold_flag(self): + args = build_parser().parse_args(["timeout-check", "999", "--threshold", "10"]) + assert args.threshold == 10.0 + + def test_timeout_check_threshold_defaults_to_none(self): + args = build_parser().parse_args(["timeout-check", "999"]) + assert args.threshold is None + + def test_timeout_check_no_ids_exits(self): + with pytest.raises(SystemExit): + build_parser().parse_args(["timeout-check"]) + + +# =========================================================================== +# build_parser — common CLI args +# =========================================================================== + + +class TestCommonCliArgs: + def test_club_ref_flag(self): + args = build_parser().parse_args(["member-summary", "--club-ref", "my-club"]) + assert args.club_ref == "my-club" + + def test_club_name_flag(self): + args = build_parser().parse_args(["member-summary", "--club-name", "My Club"]) + assert args.club_name == "My Club" + + def test_club_ref_defaults_to_none(self): + args = build_parser().parse_args(["member-summary"]) + assert args.club_ref is None + + def test_year_flag_on_match_participation(self): + args = build_parser().parse_args(["match-participation", "--year", "2025"]) + assert args.year == 2025 + + def test_clubs_flag_on_prospects(self): + args = build_parser().parse_args(["prospects", "--clubs", "club-a", "club-b"]) + assert args.clubs == ["club-a", "club-b"] + + def test_exclusion_club_flag_on_prospects(self): + args = build_parser().parse_args(["prospects", "--exclusion-club", "home-club"]) + assert args.exclusion_club == "home-club" + + def test_common_args_available_on_all_subcommands(self): + for subcmd_argv in [ + ["member-summary", "--club-ref", "x"], + ["match-participation", "--club-ref", "x"], + ["prospects", "--club-ref", "x"], + ["match-eligibility", "--club-ref", "x"], + ["timeout-check", "all", "--club-ref", "x"], + ]: + args = build_parser().parse_args(subcmd_argv) + assert args.club_ref == "x" + + +# =========================================================================== +# main() — CLI override precedence +# =========================================================================== + + +class TestCliOverridePrecedence: + def test_club_ref_cli_overrides_env(self): + config = _make_config(club_ref="env-club") + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.MemberSummaryReport", return_value=mock_instance) as mock_cls, + ): + main(["member-summary", "--club-ref", "cli-club"]) + + passed_config = mock_cls.call_args[0][1] + assert passed_config.club_ref == "cli-club" + + def test_env_used_when_no_cli_flag(self): + config = _make_config(club_ref="env-club") + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.MemberSummaryReport", return_value=mock_instance) as mock_cls, + ): + main(["member-summary"]) + + passed_config = mock_cls.call_args[0][1] + assert passed_config.club_ref == "env-club" + + def test_year_cli_overrides_env(self): + config = _make_config(data_analysis_year=2024) + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.MatchParticipationReport", return_value=mock_instance) as mock_cls, + ): + main(["match-participation", "--year", "2025"]) + + passed_config = mock_cls.call_args[0][1] + assert passed_config.data_analysis_year == 2025 + + +# =========================================================================== +# main() — timeout-check routing +# =========================================================================== + + +class TestTimeoutCheckRouting: + def test_timeout_check_runs_correct_report(self): + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + mock_instance.format_console_summary.return_value = "No issues" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=_make_config()), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.TimeoutCheckReport", return_value=mock_instance) as mock_cls, + ): + main(["timeout-check", "999"]) + + mock_cls.assert_called_once() + mock_instance.run.assert_called_once() + + def test_timeout_check_prints_console_summary(self, capsys): + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + mock_instance.format_console_summary.return_value = "Summary text here" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=_make_config()), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.TimeoutCheckReport", return_value=mock_instance), + ): + main(["timeout-check", "999"]) + + captured = capsys.readouterr() + assert "Summary text here" in captured.out + + def test_timeout_check_passes_match_ids(self): + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + mock_instance.format_console_summary.return_value = "" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=_make_config()), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.TimeoutCheckReport", return_value=mock_instance) as mock_cls, + ): + main(["timeout-check", "111", "222"]) + + _, kwargs = mock_cls.call_args + assert kwargs["match_ids"] == ["111", "222"] + + def test_timeout_check_threshold_overrides_config(self): + config = _make_config() + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + mock_instance.format_console_summary.return_value = "" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.TimeoutCheckReport", return_value=mock_instance) as mock_cls, + ): + main(["timeout-check", "999", "--threshold", "24"]) + + passed_config = mock_cls.call_args[0][1] + assert passed_config.timeout_threshold_hours == 24.0 diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index dfd6210..61386c2 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -222,6 +222,49 @@ def test_empty_string_normalised_to_none(self, monkeypatch): assert cfg.exclusion_club is None +# =========================================================================== +# Optional field — timeout_threshold_hours +# =========================================================================== + + +class TestTimeoutThresholdHours: + def test_absent_defaults_to_5(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.delenv("TIMEOUT_THRESHOLD_HOURS", raising=False) + cfg = AppConfig.from_env() + assert cfg.timeout_threshold_hours == 5.0 + + def test_parsed_to_float(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "3.5") + cfg = AppConfig.from_env() + assert cfg.timeout_threshold_hours == 3.5 + + def test_integer_value_parsed(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "10") + cfg = AppConfig.from_env() + assert cfg.timeout_threshold_hours == 10.0 + + def test_whitespace_stripped_then_parsed(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", " 7.5 ") + cfg = AppConfig.from_env() + assert cfg.timeout_threshold_hours == 7.5 + + def test_empty_string_defaults_to_5(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "") + cfg = AppConfig.from_env() + assert cfg.timeout_threshold_hours == 5.0 + + def test_non_numeric_raises(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "abc") + with pytest.raises(ValueError, match="TIMEOUT_THRESHOLD_HOURS"): + AppConfig.from_env() + + # =========================================================================== # Full happy path # =========================================================================== @@ -235,6 +278,7 @@ def test_all_fields_present(self, monkeypatch): monkeypatch.setenv("MATCH_ID", "99999") monkeypatch.setenv("LIST_OF_CLUBS", "team-ireland,team-england") monkeypatch.setenv("EXCLUSION_CLUB", "team-scotland") + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "10") cfg = AppConfig.from_env() assert cfg.club_ref == "team-scotland" assert cfg.club_name == "Team Scotland" @@ -242,6 +286,7 @@ def test_all_fields_present(self, monkeypatch): assert cfg.match_id == "99999" assert cfg.prospect_clubs == ["team-ireland", "team-england"] assert cfg.exclusion_club == "team-scotland" + assert cfg.timeout_threshold_hours == 10.0 def test_returns_appconfig_instance(self, monkeypatch): _set_required(monkeypatch) diff --git a/tests/unit/test_excel.py b/tests/unit/test_excel.py index 5750602..6b4c960 100644 --- a/tests/unit/test_excel.py +++ b/tests/unit/test_excel.py @@ -53,9 +53,7 @@ def test_write_creates_output_dir_if_absent(self, tmp_path): output_dir = tmp_path / "deep" / "nested" / "dir" assert not output_dir.exists() sheet = SheetConfig(name="Data", dataframe=_simple_df()) - ExcelReportWriter( - output_dir=str(output_dir), base_name="R", sheets=[sheet] - ).write() + ExcelReportWriter(output_dir=str(output_dir), base_name="R", sheets=[sheet]).write() assert output_dir.exists() def test_write_empty_dataframe_still_creates_file(self, tmp_path): diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index cc3ea8c..9218e82 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -59,7 +59,9 @@ def test_last_online_is_utc_datetime(self, player_profile_response, player_stats assert isinstance(member.last_online, datetime) assert member.last_online == datetime.fromtimestamp(1700000000, tz=UTC) - def test_joined_club_none_when_not_provided(self, player_profile_response, player_stats_response): + def test_joined_club_none_when_not_provided( + self, player_profile_response, player_stats_response + ): member = Member.from_api_response( profile=player_profile_response, stats=player_stats_response, @@ -125,9 +127,7 @@ def test_no_fide_title_is_empty_string(self, player_stats_response): member = Member.from_api_response(profile=profile, stats=player_stats_response) assert member.fide_title == "" - def test_timeout_percent_defaults_to_zero_when_record_missing( - self, player_profile_response - ): + def test_timeout_percent_defaults_to_zero_when_record_missing(self, player_profile_response): stats = {"chess_daily": {"last": {"rating": 1200}}} member = Member.from_api_response(profile=player_profile_response, stats=stats) assert member.timeout_percent == 0.0 @@ -211,9 +211,7 @@ def test_chess960_detected_via_variant_field(self): assert match.variant == "chess960" def test_standard_chess_variant(self, match_detail_response_team1): - match = Match.from_api_response( - match_detail_response_team1, "1", "http://x", "Test Club" - ) + match = Match.from_api_response(match_detail_response_team1, "1", "http://x", "Test Club") assert match.variant == "chess" def test_no_max_rating_returns_none(self): @@ -245,7 +243,12 @@ def test_club_not_in_match_returns_empty_participants(self): "name": "Other Match", "settings": {"rules": "chess", "variant": ""}, "teams": { - "team1": {"name": "Club A", "players": [{"username": "x", "played_as_white": "win", "played_as_black": "win"}]}, + "team1": { + "name": "Club A", + "players": [ + {"username": "x", "played_as_white": "win", "played_as_black": "win"} + ], + }, "team2": {"name": "Club B", "players": []}, }, } diff --git a/tests/unit/test_reports.py b/tests/unit/test_reports.py index bb2246f..e66f100 100644 --- a/tests/unit/test_reports.py +++ b/tests/unit/test_reports.py @@ -14,6 +14,8 @@ from __future__ import annotations import os +from datetime import UTC, datetime +from datetime import timedelta as td from unittest.mock import MagicMock, patch import pytest @@ -23,6 +25,7 @@ from chesscom.reports.match_participation import MatchParticipationReport from chesscom.reports.member_summary import MemberSummaryReport from chesscom.reports.prospect import ProspectReport +from chesscom.reports.timeout_check import TimeoutCheckReport # --------------------------------------------------------------------------- # Shared fixtures / factories @@ -130,9 +133,17 @@ def test_collect_data_returns_one_row_per_member(self): def test_collect_data_has_expected_columns(self): r = MemberSummaryReport(_make_client(), _make_config()) row = r.collect_data()[0] - for col in ("FIDE Title", "Username", "Name", "Joined Chess.com", - "Joined Club", "Last Online", "Daily Rating", - "Chess960 Rating", "Timeout Percentage"): + for col in ( + "FIDE Title", + "Username", + "Name", + "Joined Chess.com", + "Joined Club", + "Last Online", + "Daily Rating", + "Chess960 Rating", + "Timeout Percentage", + ): assert col in row, f"Missing column: {col}" def test_collect_data_daily_rating(self): @@ -220,9 +231,17 @@ def test_no_prospect_clubs_returns_empty(self): def test_collect_data_columns(self): r = ProspectReport(_make_client(), _make_config()) row = r.collect_data()[0] - for col in ("FIDE Title", "Username", "Name", "Sourced Club", - "Daily Rating", "Chess960 Rating", "Timeout Percentage", - "Last Online", "Joined Chess.com"): + for col in ( + "FIDE Title", + "Username", + "Name", + "Sourced Club", + "Daily Rating", + "Chess960 Rating", + "Timeout Percentage", + "Last Online", + "Joined Chess.com", + ): assert col in row, f"Missing column: {col}" def test_full_data_not_fetched_for_excluded_member(self): @@ -257,8 +276,15 @@ def test_collect_data_returns_rows(self): def test_collect_data_columns(self): r = MatchEligibilityReport(_make_client(), _make_config()) row = r.collect_data()[0] - for col in ("Username", "Daily Rating", "Chess960 Rating", - "Variant", "Last Online", "Timeout Percentage", "Signed Up"): + for col in ( + "Username", + "Daily Rating", + "Chess960 Rating", + "Variant", + "Last Online", + "Timeout Percentage", + "Signed Up", + ): assert col in row, f"Missing column: {col}" def test_collect_data_raises_when_no_match_id(self): @@ -291,9 +317,7 @@ def test_member_above_max_rating_excluded(self): **_MATCH_DATA, "settings": {"max_rating": 1000, "rules": "chess"}, } - r = MatchEligibilityReport( - _make_client(match_data=match_data), _make_config() - ) + r = MatchEligibilityReport(_make_client(match_data=match_data), _make_config()) data = r.collect_data() # alice's 1500 > 1000 → no rows assert data == [] @@ -309,9 +333,7 @@ def test_chess960_variant_detected(self): **_MATCH_DATA, "settings": {"max_rating": 1800, "rules": "chess960"}, } - r = MatchEligibilityReport( - _make_client(match_data=match_data), _make_config() - ) + r = MatchEligibilityReport(_make_client(match_data=match_data), _make_config()) data = r.collect_data() assert data[0]["Variant"] == "CHESS960" @@ -321,9 +343,7 @@ def test_no_max_rating_includes_all_members(self): **_MATCH_DATA, "settings": {"rules": "chess"}, # no max_rating key } - r = MatchEligibilityReport( - _make_client(match_data=match_data), _make_config() - ) + r = MatchEligibilityReport(_make_client(match_data=match_data), _make_config()) data = r.collect_data() assert len(data) == 1 # alice still included @@ -349,10 +369,18 @@ def test_collect_data_returns_member_rows(self): def test_collect_data_columns(self): r = MatchParticipationReport(_make_client(), _make_config()) row = r.collect_data()[0] - for col in ("Username", "Daily Rating", "Joined Chess.com", - "Joined Club", "Last Online", "Timeout Percentage", - "Club Timeouts", "Total Matches", - "Participation %", "Win Rate %"): + for col in ( + "Username", + "Daily Rating", + "Joined Chess.com", + "Joined Club", + "Last Online", + "Timeout Percentage", + "Club Timeouts", + "Total Matches", + "Participation %", + "Win Rate %", + ): assert col in row, f"Missing column: {col}" def test_collect_data_raises_when_no_year(self): @@ -365,12 +393,12 @@ def test_collect_data_filters_matches_by_year(self): """Matches from 2023 should be excluded when year=2024.""" club_matches = { "finished": [ - { # 2023 match — should be excluded + { # 2023 match — should be excluded "@id": "https://api.chess.com/pub/match/100", "name": "Old Match", "start_time": 1_672_531_200, # 2023-01-01 }, - { # 2024 match — should be included + { # 2024 match — should be included "@id": "https://api.chess.com/pub/match/999", "name": "League Match 1", "start_time": 1_704_067_200, # 2024-01-01 @@ -475,3 +503,209 @@ def test_match_participation_run(self, tmp_path): r = MatchParticipationReport(_make_client(), _make_config()) path = _run_with_tmpdir(r, tmp_path) assert os.path.isfile(path) + + def test_timeout_check_run(self, tmp_path): + client = _make_timeout_client() + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + path = _run_with_tmpdir(r, tmp_path) + assert os.path.isfile(path) + + +# =========================================================================== +# TimeoutCheckReport — test data +# =========================================================================== + +_IN_PROGRESS_MATCH_DATA = { + "name": "League Match Live", + "start_time": 1_704_067_200, + "settings": {"rules": "chess"}, + "teams": { + "team1": { + "name": "Team Scotland", + "players": [ + { + "username": "alice", + "board": "https://api.chess.com/pub/match/999/1", + "played_as_white": "timeout", + "played_as_black": "win", + }, + { + "username": "bob", + "board": "https://api.chess.com/pub/match/999/2", + # No played_as_white/black — both games in progress + }, + ], + }, + "team2": {"name": "Opponents", "players": []}, + }, +} + +_NO_TIMEOUT_MATCH_DATA = { + "name": "Clean Match", + "start_time": 1_704_067_200, + "settings": {"rules": "chess"}, + "teams": { + "team1": { + "name": "Team Scotland", + "players": [ + { + "username": "alice", + "board": "https://api.chess.com/pub/match/999/1", + "played_as_white": "win", + "played_as_black": "win", + }, + ], + }, + "team2": {"name": "Opponents", "players": []}, + }, +} + +_CLUB_MATCHES_IN_PROGRESS = { + "finished": [], + "in_progress": [ + { + "@id": "https://api.chess.com/pub/match/999", + "name": "League Match Live", + "start_time": 1_704_067_200, + } + ], + "registered": [], +} + + +def _make_board_data(hours_from_now: float) -> dict: + """Build board data with a game whose move_by is *hours_from_now* away.""" + move_by_ts = int((datetime.now(tz=UTC) + td(hours=hours_from_now)).timestamp()) + return { + "board_scores": {}, + "games": [ + { + "white": { + "username": "bob", + "team": "https://api.chess.com/pub/club/team-scotland", + }, + "black": { + "username": "opponent1", + "team": "https://api.chess.com/pub/club/opponents", + }, + "turn": "white", + "move_by": move_by_ts, + "time_control": "1/259200", + "time_class": "daily", + "rules": "chess", + } + ], + } + + +def _make_timeout_client(match_data=None, board_data=None, club_matches=None): + client = MagicMock() + client.get_match.return_value = match_data or _IN_PROGRESS_MATCH_DATA + client.get_match_board.return_value = board_data or _make_board_data(2) + client.get_club_matches.return_value = club_matches or _CLUB_MATCHES_IN_PROGRESS + return client + + +# =========================================================================== +# TimeoutCheckReport +# =========================================================================== + + +class TestTimeoutCheckReport: + def test_get_report_name(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + assert r.get_report_name() == "Timeout Check Report" + + def test_collect_data_detects_completed_timeout(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + data = r.collect_data() + timed_out = [row for row in data if row["Status"] == "Timed Out"] + assert len(timed_out) >= 1 + assert timed_out[0]["Username"] == "alice" + assert timed_out[0]["Colour"] == "white" + + def test_collect_data_detects_at_risk(self): + # bob has no results + board shows move_by 2h away, threshold 5h + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + data = r.collect_data() + at_risk = [row for row in data if row["Status"] == "At Risk"] + assert len(at_risk) >= 1 + assert at_risk[0]["Username"] == "bob" + + def test_collect_data_no_at_risk_when_safe(self): + # move_by is 48h away, threshold 5h → not at risk + client = _make_timeout_client(board_data=_make_board_data(48)) + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + data = r.collect_data() + at_risk = [row for row in data if row["Status"] == "At Risk"] + assert len(at_risk) == 0 + + def test_collect_data_no_alerts_for_clean_match(self): + client = _make_timeout_client(match_data=_NO_TIMEOUT_MATCH_DATA) + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + data = r.collect_data() + assert len(data) == 0 + + def test_build_sheet_configs_produces_two_sheets(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + data = r.collect_data() + configs = r.build_sheet_configs(data) + assert len(configs) == 2 + names = [c.name for c in configs] + assert "Timeout Alerts by Match" in names + assert "Timeout Summary by Player" in names + + def test_player_summary_sheet_counts_timeouts(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + data = r.collect_data() + configs = r.build_sheet_configs(data) + player_sheet = next(c for c in configs if c.name == "Timeout Summary by Player") + df = player_sheet.dataframe + # alice has 1 completed timeout + alice_rows = df[df["Username"] == "alice"] + assert len(alice_rows) == 1 + assert alice_rows.iloc[0]["Total Timeouts"] == 1 + + def test_format_console_summary_no_alerts(self): + client = _make_timeout_client(match_data=_NO_TIMEOUT_MATCH_DATA) + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + r.collect_data() + summary = r.format_console_summary() + assert "No timeout issues found" in summary + + def test_format_console_summary_with_alerts(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + r.collect_data() + summary = r.format_console_summary() + assert "TIMED OUT" in summary + assert "alice" in summary + assert "League Match Live" in summary + + def test_all_keyword_fetches_in_progress(self): + client = _make_timeout_client() + r = TimeoutCheckReport(client, _make_config(), match_ids=["all"]) + r.collect_data() + client.get_club_matches.assert_called_once_with("team-scotland") + + def test_specific_ids_do_not_fetch_club_matches(self): + client = _make_timeout_client() + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + r.collect_data() + client.get_club_matches.assert_not_called() + + def test_collect_data_has_expected_columns(self): + r = TimeoutCheckReport(_make_timeout_client(), _make_config(), match_ids=["999"]) + data = r.collect_data() + assert len(data) > 0 + row = data[0] + for col in ( + "Match Name", + "Match ID", + "Username", + "Board", + "Colour", + "Status", + "Move By", + "Hours Remaining", + ): + assert col in row, f"Missing column: {col}" diff --git a/tests/unit/test_services.py b/tests/unit/test_services.py index aa654ac..8ca5952 100644 --- a/tests/unit/test_services.py +++ b/tests/unit/test_services.py @@ -10,18 +10,21 @@ from __future__ import annotations from datetime import UTC, datetime +from datetime import timedelta as td import pytest from chesscom.domain.models import Match, MatchResult, Member, MemberParticipation from chesscom.domain.services import ( build_participation_stats, + calculate_hours_remaining, calculate_participation_percentage, calculate_win_rate, classify_result, deduplicate_members, exclude_members, filter_members_by_rating, + is_timeout_risk, ) # --------------------------------------------------------------------------- @@ -388,3 +391,57 @@ def test_returns_member_participation_instance(self): member = _member("alice") stats = build_participation_stats(member, []) assert isinstance(stats, MemberParticipation) + + +# --------------------------------------------------------------------------- +# calculate_hours_remaining +# --------------------------------------------------------------------------- + + +class TestCalculateHoursRemaining: + def test_future_deadline_returns_positive(self): + move_by = datetime.now(tz=UTC) + td(hours=2) + result = calculate_hours_remaining(move_by) + assert abs(result - 2.0) < 0.05 + + def test_past_deadline_returns_negative(self): + move_by = datetime.now(tz=UTC) - td(hours=1) + result = calculate_hours_remaining(move_by) + assert abs(result - (-1.0)) < 0.05 + + def test_returns_float(self): + move_by = datetime.now(tz=UTC) + td(hours=5) + assert isinstance(calculate_hours_remaining(move_by), float) + + def test_result_rounded_to_two_decimal_places(self): + move_by = datetime.now(tz=UTC) + td(hours=3, minutes=20) + result = calculate_hours_remaining(move_by) + assert result == round(result, 2) + + +# --------------------------------------------------------------------------- +# is_timeout_risk +# --------------------------------------------------------------------------- + + +class TestIsTimeoutRisk: + def test_within_threshold_returns_true(self): + move_by = datetime.now(tz=UTC) + td(hours=2) + assert is_timeout_risk(move_by, threshold_hours=5) is True + + def test_beyond_threshold_returns_false(self): + move_by = datetime.now(tz=UTC) + td(hours=10) + assert is_timeout_risk(move_by, threshold_hours=5) is False + + def test_past_deadline_returns_true(self): + move_by = datetime.now(tz=UTC) - td(hours=1) + assert is_timeout_risk(move_by, threshold_hours=5) is True + + def test_at_threshold_returns_true(self): + # Exactly at threshold (within tolerance) + move_by = datetime.now(tz=UTC) + td(hours=5) + assert is_timeout_risk(move_by, threshold_hours=5) is True + + def test_just_beyond_threshold_returns_false(self): + move_by = datetime.now(tz=UTC) + td(hours=5, minutes=5) + assert is_timeout_risk(move_by, threshold_hours=5) is False From 82a845773c2e5384c4c93f25aec10b011ac1b593 Mon Sep 17 00:00:00 2001 From: ldastey-dev Date: Sun, 7 Jun 2026 13:13:46 +0100 Subject: [PATCH 2/5] fix: address review findings from security, principal, architect, and DevOps reviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes applied: - HIGH: Exact slug matching in _is_our_player() — parse final URL path segment instead of substring match to prevent team-scotland matching team-scotland-juniors - MEDIUM: Surface board fetch failures — catch requests.RequestException specifically, track skipped boards, show warning in console summary - MEDIUM: CLI override semantics — use 'is not None' consistently in _apply_cli_overrides() to match documented behaviour - MEDIUM: Domain purity — inject optional 'now' parameter into calculate_hours_remaining() and is_timeout_risk() for deterministic testing - LOW: Threshold validation — reject negative, zero, NaN, and Inf values for TIMEOUT_THRESHOLD_HOURS - LOW: Fix stale AGENTS.md reference (4 → 5 concrete reports) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 2 +- src/chesscom/cli.py | 10 ++-- src/chesscom/config.py | 6 +++ src/chesscom/domain/services.py | 22 +++++--- src/chesscom/reports/timeout_check.py | 75 ++++++++++++++++----------- tests/unit/test_config.py | 24 +++++++++ tests/unit/test_reports.py | 24 +++++++++ 7 files changed, 122 insertions(+), 41 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d50e1af..e3360aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,7 +74,7 @@ Dependencies point inward. This is non-negotiable. ```text Presentation cli.py, __main__.py ↓ -Application reports/*.py (BaseReport ABC + 4 concrete reports) +Application reports/*.py (BaseReport ABC + 5 concrete reports) ↓ Domain domain/models.py, domain/services.py ↓ diff --git a/src/chesscom/cli.py b/src/chesscom/cli.py index 0a9e1a2..e0c606c 100644 --- a/src/chesscom/cli.py +++ b/src/chesscom/cli.py @@ -55,17 +55,17 @@ def _apply_cli_overrides(config: AppConfig, args: argparse.Namespace) -> AppConf """ overrides: dict = {} - if getattr(args, "club_ref", None): + if getattr(args, "club_ref", None) is not None: overrides["club_ref"] = args.club_ref - if getattr(args, "club_name", None): + if getattr(args, "club_name", None) is not None: overrides["club_name"] = args.club_name - if getattr(args, "match_id", None): + if getattr(args, "match_id", None) is not None: overrides["match_id"] = args.match_id if getattr(args, "year", None) is not None: overrides["data_analysis_year"] = args.year - if getattr(args, "clubs", None): + if getattr(args, "clubs", None) is not None: overrides["prospect_clubs"] = args.clubs - if getattr(args, "exclusion_club", None): + if getattr(args, "exclusion_club", None) is not None: overrides["exclusion_club"] = args.exclusion_club if getattr(args, "threshold", None) is not None: overrides["timeout_threshold_hours"] = args.threshold diff --git a/src/chesscom/config.py b/src/chesscom/config.py index a4c2913..715e20c 100644 --- a/src/chesscom/config.py +++ b/src/chesscom/config.py @@ -117,6 +117,12 @@ def from_env(cls) -> AppConfig: raise ValueError( f"TIMEOUT_THRESHOLD_HOURS must be a number; got '{raw_threshold}'" ) from exc + import math + + if not math.isfinite(timeout_threshold_hours) or timeout_threshold_hours <= 0: + raise ValueError( + f"TIMEOUT_THRESHOLD_HOURS must be a positive number; got '{raw_threshold}'" + ) return cls( club_ref=club_ref, diff --git a/src/chesscom/domain/services.py b/src/chesscom/domain/services.py index 0b6bf24..9d0ae66 100644 --- a/src/chesscom/domain/services.py +++ b/src/chesscom/domain/services.py @@ -257,31 +257,41 @@ def build_participation_stats( # --------------------------------------------------------------------------- -def calculate_hours_remaining(move_by: datetime) -> float: - """Return the number of hours between now and *move_by*. +def calculate_hours_remaining(move_by: datetime, now: datetime | None = None) -> float: + """Return the number of hours between *now* and *move_by*. A negative value means the deadline has already passed. Args: move_by: UTC datetime when the next move must be made. + now: Reference time. Defaults to ``datetime.now(tz=UTC)`` when + ``None`` — pass explicitly in tests for determinism. Returns: Hours remaining as a float, rounded to two decimal places. """ - delta = move_by - datetime.now(tz=UTC) + if now is None: + now = datetime.now(tz=UTC) + delta = move_by - now return round(delta.total_seconds() / 3600, 2) -def is_timeout_risk(move_by: datetime, threshold_hours: float) -> bool: - """Return ``True`` when *move_by* is within *threshold_hours* of now. +def is_timeout_risk( + move_by: datetime, + threshold_hours: float, + now: datetime | None = None, +) -> bool: + """Return ``True`` when *move_by* is within *threshold_hours* of *now*. A ``move_by`` that has already passed is always considered at risk. Args: move_by: UTC datetime when the next move must be made. threshold_hours: Maximum acceptable hours remaining. + now: Reference time. Defaults to ``datetime.now(tz=UTC)`` when + ``None``. Returns: ``True`` if the remaining time is at or below the threshold. """ - return calculate_hours_remaining(move_by) <= threshold_hours + return calculate_hours_remaining(move_by, now) <= threshold_hours diff --git a/src/chesscom/reports/timeout_check.py b/src/chesscom/reports/timeout_check.py index 4b96025..18618c2 100644 --- a/src/chesscom/reports/timeout_check.py +++ b/src/chesscom/reports/timeout_check.py @@ -13,6 +13,7 @@ from datetime import UTC, datetime import pandas as pd +import requests from chesscom.domain.models import TimeoutAlert from chesscom.domain.services import calculate_hours_remaining, is_timeout_risk @@ -45,6 +46,7 @@ def __init__(self, client, config, *, match_ids: list[str]) -> None: super().__init__(client, config) self._match_ids = match_ids self._alerts: list[TimeoutAlert] = [] + self._skipped_boards: list[str] = [] def get_report_name(self) -> str: return "Timeout Check Report" @@ -64,6 +66,7 @@ def collect_data(self) -> list[dict]: """ raw_matches = self._resolve_matches() alerts: list[TimeoutAlert] = [] + self._skipped_boards = [] for raw in raw_matches: url: str = raw.get("@id", "") @@ -195,33 +198,45 @@ def format_console_summary(self) -> str: Returns: Formatted multi-line string suitable for printing to stdout. """ - if not self._alerts: + if not self._alerts and not self._skipped_boards: return "No timeout issues found across checked matches." - # Group alerts by match - by_match: dict[str, list[TimeoutAlert]] = {} - for alert in self._alerts: - key = f"{alert.match_name} (ID: {alert.match_id})" - by_match.setdefault(key, []).append(alert) - - lines: list[str] = ["=== Timeout Check Summary ===", ""] - for match_label, match_alerts in by_match.items(): - timed_out = [a for a in match_alerts if a.status == "timed_out"] - at_risk = [a for a in match_alerts if a.status == "at_risk"] - lines.append(f"Match: {match_label}") - - parts: list[str] = [] - if at_risk: - parts.append(f"{len(at_risk)} at risk") - if timed_out: - parts.append(f"{len(timed_out)} timed out") - lines.append(f" {', '.join(parts)}") - - for a in timed_out: - lines.append(f" TIMED OUT: {a.username} ({a.colour})") - for a in at_risk: - hrs = f"{a.hours_remaining:.1f}" if a.hours_remaining is not None else "?" - lines.append(f" AT RISK: {a.username} ({a.colour}) — {hrs} hours remaining") + lines: list[str] = [] + + if self._alerts: + # Group alerts by match + by_match: dict[str, list[TimeoutAlert]] = {} + for alert in self._alerts: + key = f"{alert.match_name} (ID: {alert.match_id})" + by_match.setdefault(key, []).append(alert) + + lines.extend(["=== Timeout Check Summary ===", ""]) + for match_label, match_alerts in by_match.items(): + timed_out = [a for a in match_alerts if a.status == "timed_out"] + at_risk = [a for a in match_alerts if a.status == "at_risk"] + lines.append(f"Match: {match_label}") + + parts: list[str] = [] + if at_risk: + parts.append(f"{len(at_risk)} at risk") + if timed_out: + parts.append(f"{len(timed_out)} timed out") + lines.append(f" {', '.join(parts)}") + + for a in timed_out: + lines.append(f" TIMED OUT: {a.username} ({a.colour})") + for a in at_risk: + hrs = f"{a.hours_remaining:.1f}" if a.hours_remaining is not None else "?" + lines.append(f" AT RISK: {a.username} ({a.colour}) — {hrs} hours remaining") + lines.append("") + else: + lines.append("No timeout issues found across checked matches.") + lines.append("") + + if self._skipped_boards: + lines.append(f"Warning: {len(self._skipped_boards)} board(s) could not be checked:") + for url in self._skipped_boards: + lines.append(f" - {url}") lines.append("") return "\n".join(lines) @@ -257,7 +272,8 @@ def _check_board_for_risks( """Fetch board data and append at-risk alerts for our team's players.""" try: board_data = self.client.get_match_board(board_url) - except Exception: + except requests.RequestException: + self._skipped_boards.append(board_url) return club_ref_lower = self.config.club_ref.lower() @@ -305,9 +321,10 @@ def _check_board_for_risks( @staticmethod def _is_our_player(team_url: str, club_ref_lower: str) -> bool: """Check if a player belongs to our club using the team URL.""" - if team_url: - return club_ref_lower in team_url.lower() - return False + if not team_url: + return False + slug = team_url.rstrip("/").rsplit("/", 1)[-1].lower() + return slug == club_ref_lower @staticmethod def _alerts_to_rows(alerts: list[TimeoutAlert]) -> list[dict]: diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 61386c2..bbd3809 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -264,6 +264,30 @@ def test_non_numeric_raises(self, monkeypatch): with pytest.raises(ValueError, match="TIMEOUT_THRESHOLD_HOURS"): AppConfig.from_env() + def test_negative_value_raises(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "-5") + with pytest.raises(ValueError, match="positive"): + AppConfig.from_env() + + def test_zero_value_raises(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "0") + with pytest.raises(ValueError, match="positive"): + AppConfig.from_env() + + def test_infinity_raises(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "inf") + with pytest.raises(ValueError, match="positive"): + AppConfig.from_env() + + def test_nan_raises(self, monkeypatch): + _set_required(monkeypatch) + monkeypatch.setenv("TIMEOUT_THRESHOLD_HOURS", "nan") + with pytest.raises(ValueError, match="positive"): + AppConfig.from_env() + # =========================================================================== # Full happy path diff --git a/tests/unit/test_reports.py b/tests/unit/test_reports.py index e66f100..f80c093 100644 --- a/tests/unit/test_reports.py +++ b/tests/unit/test_reports.py @@ -709,3 +709,27 @@ def test_collect_data_has_expected_columns(self): "Hours Remaining", ): assert col in row, f"Missing column: {col}" + + def test_at_risk_not_flagged_for_similar_club_slug(self): + """team-scotland should not match team-scotland-juniors.""" + board_data = _make_board_data(2) + board_data["games"][0]["white"]["team"] = ( + "https://api.chess.com/pub/club/team-scotland-juniors" + ) + client = _make_timeout_client(board_data=board_data) + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + data = r.collect_data() + at_risk = [row for row in data if row["Status"] == "At Risk"] + assert len(at_risk) == 0 + + def test_board_fetch_failure_recorded_in_summary(self): + """Failed board fetches should appear as warnings in the console summary.""" + import requests as _requests + + client = _make_timeout_client() + client.get_match_board.side_effect = _requests.ConnectionError("timeout") + r = TimeoutCheckReport(client, _make_config(), match_ids=["999"]) + r.collect_data() + summary = r.format_console_summary() + assert "Warning" in summary + assert "could not be checked" in summary From 7602a9c42900c6ac936fdec9dbc6a795f2e898a3 Mon Sep 17 00:00:00 2001 From: ldastey-dev Date: Sun, 7 Jun 2026 15:11:29 +0100 Subject: [PATCH 3/5] ci: enforce 90% minimum test coverage in pipeline Add --cov-fail-under=90 to pytest command so PRs that drop below the coverage threshold will fail CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4fc186c..5384525 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,7 @@ jobs: run: ruff check . - name: Run tests with coverage - run: pytest --cov=src --cov-report=term-missing --cov-report=xml + run: pytest --cov=src --cov-report=term-missing --cov-report=xml --cov-fail-under=90 - name: Upload coverage report uses: actions/upload-artifact@v7 From b747991ff71d40ad0fdd4ec353cbe4e4ed5a3a5a Mon Sep 17 00:00:00 2001 From: ldastey-dev Date: Sun, 7 Jun 2026 15:27:43 +0100 Subject: [PATCH 4/5] feat: add TIMEOUT_MATCH_IDS env var for timeout-check Allow match IDs to be configured via .env as well as CLI positional args. CLI args take precedence when provided; falls back to TIMEOUT_MATCH_IDS env var. Raises a clear error if neither is set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .env.template | 1 + src/chesscom/cli.py | 13 ++++++++++--- src/chesscom/config.py | 11 +++++++++++ tests/unit/test_cli.py | 37 ++++++++++++++++++++++++++++++++++--- 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/.env.template b/.env.template index 2f4a54c..befdfbf 100644 --- a/.env.template +++ b/.env.template @@ -9,3 +9,4 @@ DATA_ANALYSIS_YEAR=[YEAR] # match-participation LIST_OF_CLUBS=[COMMA SEPARATED LIST OF CLUBS] # prospects EXCLUSION_CLUB=[CLUB REF] # prospects (optional — members of this club are excluded from the prospect list) TIMEOUT_THRESHOLD_HOURS=[HOURS] # timeout-check (optional — hours remaining below which a game is flagged; default: 5) +TIMEOUT_MATCH_IDS=[COMMA SEPARATED IDS] # timeout-check (optional — match IDs to check, or "all" for all in-progress) diff --git a/src/chesscom/cli.py b/src/chesscom/cli.py index e0c606c..cb8e724 100644 --- a/src/chesscom/cli.py +++ b/src/chesscom/cli.py @@ -114,7 +114,13 @@ def _handle_match_eligibility(args: argparse.Namespace) -> None: def _handle_timeout_check(args: argparse.Namespace) -> None: config = _apply_cli_overrides(AppConfig.from_env(), args) - report = TimeoutCheckReport(ChessComClient(), config, match_ids=args.match_ids) + match_ids = args.match_ids if args.match_ids else config.timeout_match_ids + if not match_ids: + raise ValueError( + "No match IDs provided. Supply them as positional arguments " + "or set TIMEOUT_MATCH_IDS in your .env file." + ) + report = TimeoutCheckReport(ChessComClient(), config, match_ids=match_ids) start = time.monotonic() path = report.run() elapsed = time.monotonic() - start @@ -280,11 +286,12 @@ def build_parser() -> argparse.ArgumentParser: ) tc_parser.add_argument( "match_ids", - nargs="+", + nargs="*", metavar="MATCH_ID", help=( 'One or more match IDs to check, or "all" to check every ' - "in-progress match for the configured club." + "in-progress match for the configured club. " + "Falls back to TIMEOUT_MATCH_IDS env var if not provided." ), ) tc_parser.add_argument( diff --git a/src/chesscom/config.py b/src/chesscom/config.py index 715e20c..c071931 100644 --- a/src/chesscom/config.py +++ b/src/chesscom/config.py @@ -25,6 +25,9 @@ Number of hours remaining below which an in-progress game is flagged as at risk of timing out. Parsed to ``float``; defaults to ``5.0`` if blank. + TIMEOUT_MATCH_IDS Comma-separated list of match IDs for the timeout-check + report. Use ``all`` to check every in-progress match. + May also be supplied via CLI positional args. """ from __future__ import annotations @@ -52,6 +55,9 @@ class AppConfig: timeout_threshold_hours: Number of hours remaining on the clock below which a game is flagged as at risk of timing out. Defaults to ``5.0``. + timeout_match_ids: List of match IDs to check in the timeout-check + report. Use ``["all"]`` to check all in-progress matches. + Empty list means no matches configured (must be supplied via CLI). """ club_ref: str @@ -61,6 +67,7 @@ class AppConfig: prospect_clubs: list[str] = field(default_factory=list) exclusion_club: str | None = field(default=None) timeout_threshold_hours: float = field(default=5.0) + timeout_match_ids: list[str] = field(default_factory=list) # ------------------------------------------------------------------ # Factory @@ -124,6 +131,9 @@ def from_env(cls) -> AppConfig: f"TIMEOUT_THRESHOLD_HOURS must be a positive number; got '{raw_threshold}'" ) + raw_match_ids = os.getenv("TIMEOUT_MATCH_IDS", "") + timeout_match_ids = [m.strip() for m in raw_match_ids.split(",") if m.strip()] + return cls( club_ref=club_ref, club_name=club_name, @@ -132,4 +142,5 @@ def from_env(cls) -> AppConfig: prospect_clubs=prospect_clubs, exclusion_club=exclusion_club, timeout_threshold_hours=timeout_threshold_hours, + timeout_match_ids=timeout_match_ids, ) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 0912cfd..f83d359 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -304,9 +304,9 @@ def test_timeout_check_threshold_defaults_to_none(self): args = build_parser().parse_args(["timeout-check", "999"]) assert args.threshold is None - def test_timeout_check_no_ids_exits(self): - with pytest.raises(SystemExit): - build_parser().parse_args(["timeout-check"]) + def test_timeout_check_no_ids_gives_empty_list(self): + args = build_parser().parse_args(["timeout-check"]) + assert args.match_ids == [] # =========================================================================== @@ -476,3 +476,34 @@ def test_timeout_check_threshold_overrides_config(self): passed_config = mock_cls.call_args[0][1] assert passed_config.timeout_threshold_hours == 24.0 + + def test_timeout_check_falls_back_to_env_match_ids(self): + """When no CLI match IDs given, uses config.timeout_match_ids.""" + config = _make_config() + config = config.__class__(**{**config.__dict__, "timeout_match_ids": ["555", "666"]}) + mock_instance = MagicMock() + mock_instance.run.return_value = "output/test.xlsx" + mock_instance.format_console_summary.return_value = "" + + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + patch("chesscom.cli.TimeoutCheckReport", return_value=mock_instance) as mock_cls, + ): + main(["timeout-check"]) + + _, kwargs = mock_cls.call_args + assert kwargs["match_ids"] == ["555", "666"] + + def test_timeout_check_no_ids_anywhere_raises(self): + """Error when no CLI args and no TIMEOUT_MATCH_IDS configured.""" + config = _make_config() + with ( + patch("chesscom.cli.load_dotenv"), + patch("chesscom.cli.AppConfig.from_env", return_value=config), + patch("chesscom.cli.ChessComClient"), + ): + with pytest.raises(SystemExit) as exc_info: + main(["timeout-check"]) + assert exc_info.value.code == 1 From 82bc5f16d880e2ef72715f411728e72655b86181 Mon Sep 17 00:00:00 2001 From: ldastey-dev Date: Sun, 7 Jun 2026 15:31:48 +0100 Subject: [PATCH 5/5] docs: update README with timeout-check and CLI args for all subcommands - Add timeout-check subcommand documentation with .env and CLI examples - Document common --club-ref and --club-name flags - Show both .env-only, CLI-only, and mixed usage for every subcommand - Update environment variables table with TIMEOUT_MATCH_IDS and TIMEOUT_THRESHOLD_HOURS - Update project structure to include timeout_check.py and TimeoutAlert - Note 90% coverage enforcement in CI section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 110 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 90 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index a07b43e..9a5480b 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # chesscom Python tools for managing a Chess.com club. Generates Excel reports covering -member statistics, match participation, prospect identification, and match -eligibility. +member statistics, match participation, prospect identification, match +eligibility, and timeout monitoring. [![CI](https://github.com/ldastey-dev/chesscom/actions/workflows/ci.yml/badge.svg)](https://github.com/ldastey-dev/chesscom/actions/workflows/ci.yml) @@ -15,7 +15,8 @@ eligibility. | `member-summary` | `Club Member Summary Report.xlsx` | Roster of all club members with ratings, Chess960 ratings, FIDE titles, join dates, and last-online dates. | | `match-participation` | ` Club Contribution Report .xlsx` | Two-sheet workbook: per-member win rate, timeout rate, and participation %; plus a match-by-match breakdown. | | `prospects` | `Member Prospects.xlsx` | De-duplicated prospect list sourced from one or more target clubs, with current members excluded. | -| `match-eligibility` | `Match Eligibility .xlsx` | Club members eligible for a specific match (within its rating cap), showing sign-up status. | +| `match-eligibility` | `Match Eligibility.xlsx` | Club members eligible for a specific match (within its rating cap), showing sign-up status. | +| `timeout-check` | `Timeout Check Report.xlsx` | Two-sheet workbook: timeout alerts grouped by match; player timeout summary. Also prints a concise summary to stdout. | --- @@ -66,13 +67,27 @@ Open `.env` and fill in your values — see [Environment Variables](#environment ## Usage -All subcommands read configuration from environment variables (or a `.env` file -in the project root). +All subcommands read configuration from environment variables (`.env` file) **and** +CLI arguments. CLI arguments take precedence over environment variables when both +are provided. + +### Common flags (available on all subcommands) + +| Flag | Overrides | Description | +|---|---|---| +| `--club-ref SLUG` | `CLUB_REF` | Club URL slug for API paths | +| `--club-name NAME` | `CLUB_NAME` | Club display name | + +--- ### Club Member Summary ```bash +# Using .env (CLUB_REF and CLUB_NAME set): python -m chesscom member-summary + +# Using CLI args only: +python -m chesscom member-summary --club-ref team-scotland --club-name "Team Scotland" ``` **Required:** `CLUB_REF`, `CLUB_NAME` @@ -82,36 +97,83 @@ python -m chesscom member-summary ### Match Participation Report ```bash +# Using .env (CLUB_REF, CLUB_NAME, DATA_ANALYSIS_YEAR set): python -m chesscom match-participation + +# Using CLI args: +python -m chesscom match-participation --club-ref team-scotland --club-name "Team Scotland" --year 2025 + +# Mixed — override year only: +python -m chesscom match-participation --year 2025 ``` -**Required:** `CLUB_REF`, `CLUB_NAME`, `DATA_ANALYSIS_YEAR` +**Required:** `CLUB_REF`, `CLUB_NAME`, `DATA_ANALYSIS_YEAR` (or `--year`) --- ### Prospect Report ```bash +# Using .env (LIST_OF_CLUBS set): python -m chesscom prospects + +# Using CLI args: +python -m chesscom prospects --clubs team-ireland team-england --exclusion-club team-scotland + +# Mixed — override exclusion club only: +python -m chesscom prospects --exclusion-club team-scotland ``` -**Required:** `LIST_OF_CLUBS` -**Optional:** `EXCLUSION_CLUB` +**Required:** `LIST_OF_CLUBS` (or `--clubs`) +**Optional:** `EXCLUSION_CLUB` (or `--exclusion-club`) --- ### Match Eligibility Report ```bash -# Match ID from MATCH_ID env var: +# Using .env (MATCH_ID set): python -m chesscom match-eligibility -# Or supply it directly: +# Using CLI args: python -m chesscom match-eligibility --match-id 12345 + +# Mixed — override match ID only: +python -m chesscom match-eligibility --match-id 67890 +``` + +**Required:** `CLUB_REF`, `CLUB_NAME`, `MATCH_ID` (or `--match-id`) + +--- + +### Timeout Check + +```bash +# Using .env (TIMEOUT_MATCH_IDS set): +python -m chesscom timeout-check + +# Using CLI args — specific matches: +python -m chesscom timeout-check 12345 67890 + +# Check all in-progress matches: +python -m chesscom timeout-check all + +# Custom threshold (flag overrides TIMEOUT_THRESHOLD_HOURS): +python -m chesscom timeout-check all --threshold 24 + +# Full CLI args (no .env needed): +python -m chesscom timeout-check all \ + --club-ref team-scotland \ + --club-name "Team Scotland" \ + --threshold 10 ``` -**Required:** `CLUB_REF`, `CLUB_NAME` -**Optional:** `MATCH_ID` (can also be passed via `--match-id`) +**Required:** `CLUB_REF`, `CLUB_NAME`, plus match IDs via CLI or `TIMEOUT_MATCH_IDS` +**Optional:** `TIMEOUT_THRESHOLD_HOURS` (or `--threshold`; default: 5 hours) + +**Output:** +- **stdout** — concise per-match summary with flagged player counts +- **xlsx** — Tab 1: alerts grouped by match; Tab 2: player timeout counts (descending) --- @@ -119,7 +181,7 @@ python -m chesscom match-eligibility --match-id 12345 ```bash python -m chesscom --help -python -m chesscom match-eligibility --help +python -m chesscom timeout-check --help ``` --- @@ -130,12 +192,17 @@ Copy `.env.template` to `.env` and populate: | Variable | Required by | Description | |---|---|---| -| `CLUB_REF` | `member-summary`, `match-participation`, `match-eligibility` | Club URL slug, e.g. `team-scotland` | -| `CLUB_NAME` | `member-summary`, `match-participation`, `match-eligibility` | Display name, e.g. `Team Scotland` | +| `CLUB_REF` | `member-summary`, `match-participation`, `match-eligibility`, `timeout-check` | Club URL slug, e.g. `team-scotland` | +| `CLUB_NAME` | `member-summary`, `match-participation`, `match-eligibility`, `timeout-check` | Display name, e.g. `Team Scotland` | | `DATA_ANALYSIS_YEAR` | `match-participation` | Four-digit year to analyse, e.g. `2025` | -| `MATCH_ID` | `match-eligibility` | Chess.com match ID (optional — also accepted via `--match-id`) | +| `MATCH_ID` | `match-eligibility` | Chess.com match ID (also accepted via `--match-id`) | | `LIST_OF_CLUBS` | `prospects` | Comma-separated club slugs to source prospects from | -| `EXCLUSION_CLUB` | `prospects` | Optional club slug whose members are excluded from the prospect list | +| `EXCLUSION_CLUB` | `prospects` | Club slug whose members are excluded from the prospect list | +| `TIMEOUT_MATCH_IDS` | `timeout-check` | Comma-separated match IDs, or `all` for every in-progress match | +| `TIMEOUT_THRESHOLD_HOURS` | `timeout-check` | Hours remaining below which a game is flagged as at risk (default: `5`) | + +All variables can also be supplied (or overridden) via CLI flags. Run any subcommand +with `--help` to see available options. --- @@ -165,7 +232,7 @@ chesscom/ │ ├── api/ │ │ └── client.py # ChessComClient — all HTTP calls │ ├── domain/ -│ │ ├── models.py # Member, Match, MatchResult, MemberParticipation +│ │ ├── models.py # Member, Match, MatchResult, MemberParticipation, TimeoutAlert │ │ └── services.py # Pure calculation and filtering functions │ ├── export/ │ │ └── excel.py # ExcelReportWriter + SheetConfig @@ -174,7 +241,8 @@ chesscom/ │ ├── match_eligibility.py │ ├── match_participation.py │ ├── member_summary.py -│ └── prospect.py +│ ├── prospect.py +│ └── timeout_check.py ├── tests/ │ ├── integration/ # Live-API tests (require network) │ └── unit/ # Fast, dependency-free unit tests @@ -225,7 +293,7 @@ ruff format . The GitHub Actions workflow (`.github/workflows/ci.yml`) runs on every push and pull request to `master`. It runs `ruff check` and `pytest --cov` against -Python 3.11 and 3.12. +Python 3.11 and 3.12, enforcing a minimum 90% test coverage threshold. --- @@ -235,3 +303,5 @@ Python 3.11 and 3.12. with exponential back-off on transient failures. - A Chrome browser `User-Agent` header is used to ensure compatibility with the Chess.com API. +- The `timeout-check` command will warn if any boards could not be checked due + to API errors.