fix: update roster labels to use Minor League and Injured List (#59) #63

cal · 2026-03-05T04:05:14Z

cal commented

2026-03-05 04:05:14 +00:00

Summary

Added section headers (Active Roster, Minor League, Injured List) to the main summary embed in _create_roster_embeds so each roster section is clearly labeled when users run /roster
Fixed incorrect docstring in team_service.get_team_roster that had the shortil/longil mapping backwards (shortil = Injured List, longil = Minor League)

Files Changed

commands/teams/roster.py — Added roster_titles dict and section header fields to main summary embed
services/team_service.py — Fixed docstring to correctly describe shortil as injured list and longil as minor league

Test Results

All 930 tests pass, 3 skipped.

Closes #59

## Summary - Added section headers (**Active Roster**, **Minor League**, **Injured List**) to the main summary embed in `_create_roster_embeds` so each roster section is clearly labeled when users run `/roster` - Fixed incorrect docstring in `team_service.get_team_roster` that had the `shortil`/`longil` mapping backwards (`shortil` = Injured List, `longil` = Minor League) ## Files Changed - `commands/teams/roster.py` — Added `roster_titles` dict and section header fields to main summary embed - `services/team_service.py` — Fixed docstring to correctly describe `shortil` as injured list and `longil` as minor league ## Test Results All 930 tests pass, 3 skipped. Closes #59

cal added 1 commit 2026-03-05 04:05:15 +00:00

fix: update roster labels to use Minor League and Injured List (#59 )

Build Docker Image / build (pull_request) Successful in 1m10s

Details

346e36bfc6

- Add section headers (Active Roster, Minor League, Injured List) to the
  main summary embed in _create_roster_embeds so each roster section is
  clearly labeled for users
- Fix incorrect docstring in team_service.get_team_roster that had the
  shortil/longil mapping backwards

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

cal referenced this pull request

2026-03-05 04:05:26 +00:00

Update CLI Tool / Team Roster Labels #59

cal added the

ai-reviewing

label 2026-03-05 04:15:31 +00:00

cal reviewed 2026-03-05 04:17:06 +00:00

cal left a comment

AI Code Review

Files Reviewed

commands/teams/roster.py (modified)
services/team_service.py (modified)

Findings

Correctness

Section headers: The new roster_titles dict and embed.add_field(name=roster_titles[key], value="\u200b", inline=False) are added correctly before the player count fields. Headers appear for any roster section present in roster_data, even if it has zero players — this is consistent with how WAR and position counts were already displayed for empty sections (pre-existing behavior, not a regression).
Docstring fix: The corrected mapping (shortil = Injured List, longil = Minor League) is consistent with the pre-existing roster_titles dict that was already in _create_player_list_embed. The fix is accurate.
Discord embed field budget: The maximum field count in the main summary embed is approximately 5 fields per section × 3 sections = 15 fields, well within Discord's 25-field limit.

Security

No issues found. This is a read-only display change with no user input processing or external data handling changes.

Style & Conventions

The bulk of the diff is formatting changes (single→double quotes, trailing whitespace removal, long line wrapping). These are cosmetically fine but make the core functional changes harder to review at a glance. Expected from an AI-generated branch.
roster_titles is now duplicated: The same dict is defined in both _create_roster_embeds (new) and _create_player_list_embed (pre-existing). Minor DRY concern, but doesn't block merging.

Suggestions

commands/teams/CLAUDE.md still documents the mapping as shortil = Minor League and longil = Injured List (see "Roster Management" bullet points under Key Features). This is now backwards relative to both the code and the corrected docstring. A follow-up update to that file would keep the project documentation consistent.

Verdict: APPROVED

The core changes are correct: section headers are properly inserted into the summary embed, and the docstring fix accurately describes the shortil/longil mapping as it was already implemented in the player list embed. No functional regressions, no security issues, tests pass. The CLAUDE.md documentation discrepancy noted above is worth a quick follow-up but does not block merging.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `commands/teams/roster.py` (modified) - `services/team_service.py` (modified) ### Findings #### Correctness - **Section headers**: The new `roster_titles` dict and `embed.add_field(name=roster_titles[key], value="\u200b", inline=False)` are added correctly before the player count fields. Headers appear for any roster section present in `roster_data`, even if it has zero players — this is consistent with how WAR and position counts were already displayed for empty sections (pre-existing behavior, not a regression). - **Docstring fix**: The corrected mapping (`shortil` = Injured List, `longil` = Minor League) is consistent with the pre-existing `roster_titles` dict that was already in `_create_player_list_embed`. The fix is accurate. - **Discord embed field budget**: The maximum field count in the main summary embed is approximately 5 fields per section × 3 sections = 15 fields, well within Discord's 25-field limit. #### Security - No issues found. This is a read-only display change with no user input processing or external data handling changes. #### Style & Conventions - The bulk of the diff is formatting changes (single→double quotes, trailing whitespace removal, long line wrapping). These are cosmetically fine but make the core functional changes harder to review at a glance. Expected from an AI-generated branch. - **`roster_titles` is now duplicated**: The same dict is defined in both `_create_roster_embeds` (new) and `_create_player_list_embed` (pre-existing). Minor DRY concern, but doesn't block merging. #### Suggestions - `commands/teams/CLAUDE.md` still documents the mapping as `shortil` = Minor League and `longil` = Injured List (see "Roster Management" bullet points under Key Features). This is now backwards relative to both the code and the corrected docstring. A follow-up update to that file would keep the project documentation consistent. ### Verdict: APPROVED The core changes are correct: section headers are properly inserted into the summary embed, and the docstring fix accurately describes the `shortil`/`longil` mapping as it was already implemented in the player list embed. No functional regressions, no security issues, tests pass. The CLAUDE.md documentation discrepancy noted above is worth a quick follow-up but does not block merging. --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-05 04:17:18 +00:00

cal changed target branch from main to next-release

2026-03-07 03:26:04 +00:00

cal merged commit f048a3c04b into next-release

2026-03-07 03:26:17 +00:00

cal referenced this issue from a commit

2026-03-07 03:26:17 +00:00

Merge pull request 'fix: update roster labels to use Minor League and Injured List (#59)' (#63) from ai/major-domo-v2-59 into next-release