cal/paper-dynasty-discord
1
0
Fork 0
Code Issues 10 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

feat(compare): /compare command — side-by-side card stat embed (Roadmap 2.5b) #165

Open
cal wants to merge 4 commits from autonomous/feat-compare-command into main
pull from: autonomous/feat-compare-command
merge into: cal:main
Conversation 1 Commits 4 Files Changed 6 +2471 -616
cal commented 2026-04-10 15:35:44 +00:00
Owner
Copy Link

Autonomous Pipeline Finding

Source: growth-po nightly sweep (2026-04-10)
Roadmap: Phase 2.5b
Category: feature / scouting depth

What

New /compare slash command: side-by-side stat embed for two cards with directional arrows highlighting deltas. Gated on matching card type.

Batter stats: Cost (overall proxy), Rarity, Running, Steal Low, Steal High, Bunting, Hit & Run

Pitcher stats: Cost (overall proxy), Rarity, Starter Rating, Relief Rating, Closer Rating, Balk, Wild Pitch

Arrow semantics:

  • ▲ — card2 is better on this stat
  • ▼ — card1 is better on this stat
  • ═ — tied or missing data

Why

Scouting depth is a Phase 2 engagement pillar. Card comparison is a staple of every competitive TCG and players currently have to eyeball stats manually. Becomes a building block for the future trade system (2.3a) — players will want to compare before accepting trades.

Implementation Notes

The database's per-card stats live in the battingcards/pitchingcards endpoints (running, steal range, starter/relief/closer ratings, balk, wild pitch). The battingcardratings endpoint with AVG/OBP/SLG per hand is team-auth-gated and not used here. Cost serves as the overall quality proxy since it is dynamically priced based on card performance.

Test Plan

  • Unit test on embed builder: type mismatch, deltas, arrows, tied values (30 tests, all pass)
  • Manual /compare on dev bot (reviewer)

Generated via Paper Dynasty autonomous pipeline (2026-04-10)

## Autonomous Pipeline Finding **Source:** growth-po nightly sweep (2026-04-10) **Roadmap:** Phase 2.5b **Category:** feature / scouting depth ## What New `/compare` slash command: side-by-side stat embed for two cards with directional arrows highlighting deltas. Gated on matching card type. **Batter stats:** Cost (overall proxy), Rarity, Running, Steal Low, Steal High, Bunting, Hit & Run **Pitcher stats:** Cost (overall proxy), Rarity, Starter Rating, Relief Rating, Closer Rating, Balk, Wild Pitch Arrow semantics: - `▲` — card2 is better on this stat - `▼` — card1 is better on this stat - `═` — tied or missing data ## Why Scouting depth is a Phase 2 engagement pillar. Card comparison is a staple of every competitive TCG and players currently have to eyeball stats manually. Becomes a building block for the future trade system (2.3a) — players will want to compare before accepting trades. ## Implementation Notes The database's per-card stats live in the battingcards/pitchingcards endpoints (running, steal range, starter/relief/closer ratings, balk, wild pitch). The battingcardratings endpoint with AVG/OBP/SLG per hand is team-auth-gated and not used here. Cost serves as the overall quality proxy since it is dynamically priced based on card performance. ## Test Plan - [x] Unit test on embed builder: type mismatch, deltas, arrows, tied values (30 tests, all pass) - [ ] Manual /compare on dev bot (reviewer) Generated via Paper Dynasty autonomous pipeline (2026-04-10)
cal added 3 commits 2026-04-10 15:35:45 +00:00
fix(gameplay): replace bare except with NoResultFound in cache-miss paths
All checks were successful
Ruff Lint / lint (pull_request) Successful in 27s
Details
f62e08889f 
In gameplay_queries.py, 10 try/except blocks used `except Exception:` to
detect SQLModel row-not-found cache misses (.one() returning no result).
This silently swallowed connection failures, attribute errors, and
programming bugs on the gameplay hot path.

Narrowed each handler to `except NoResultFound:` (sqlalchemy.exc).
Real errors now propagate instead of being misinterpreted as cache misses.

Refs: autonomous-pipeline finding analyst-2026-04-10-003
Category: stability / error_handling

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(gauntlet): post completion recap embed on 10-win gauntlet finish
All checks were successful
Ruff Lint / lint (pull_request) Successful in 13s
Details
9228d2e66c 
Adds build_gauntlet_recap_embed (sync builder) and post_gauntlet_recap
(async sender) to gauntlets.py.  Called from post_result when wins == 10.
Embed shows champion name + user mention, final record, win-progression
ladder, and full prize-distribution table with earned/unearned markers.
Gracefully skips if channel is None or any step raises.

Roadmap 2.4a — closes growth-sweep finding 2026-04-10-003.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(compare): add /compare slash command for side-by-side card comparison
All checks were successful
Ruff Lint / lint (pull_request) Successful in 13s
Details
0b8beda8b5 
Implements Roadmap 2.5b: new /compare command lets players compare two
cards of the same type (batter vs batter or pitcher vs pitcher) in a
side-by-side embed with directional delta arrows (▲▼═).

- cogs/compare.py: new CompareCog with /compare slash command and
  player_autocomplete on both params; fetches battingcard/pitchingcard
  data from API; validates type compatibility; sends public embed
- tests/test_compare_command.py: 30 unit tests covering _delta_arrow,
  _is_pitcher, batter/pitcher embed builders, type mismatch error, and
  edge cases (None stats, tied values)
- paperdynasty.py: registers cogs.compare in COGS list

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
autonomous
size:S
type:feature
 labels 2026-04-10 15:36:05 +00:00
Claude added the
ai-reviewing
 label 2026-04-10 15:57:05 +00:00
Claude removed the
ai-reviewing
 label 2026-04-10 16:02:06 +00:00
Claude added the
ai-reviewing
 label 2026-04-10 16:52:51 +00:00
Claude requested changes 2026-04-10 16:56:32 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • cogs/compare.py (added — 425 lines, /compare cog)
  • gauntlets.py (modified — gauntlet recap embed)
  • in_game/gameplay_queries.py (modified — NoResultFound narrowing + reformatting)
  • paperdynasty.py (modified — cog registration)
  • tests/test_compare_command.py (added)
  • tests/test_gauntlet_recap.py (added)

Findings

Correctness

Bug: (loss_max or 99) falsy-zero trap — gauntlets.py:build_gauntlet_recap_embed

marker = "✅" if earned else "❌" if losses > (loss_max or 99) else "⬜"

When loss_max == 0 (perfect-run-only reward), Python evaluates 0 or 99 == 99. So for any completed run with losses (e.g. 10-1), the marker for the 10-0 reward evaluates as "❌" if 1 > 99 → False → "⬜". A player who went 10-1 sees ⬜ (pending/not yet reached) for an unearnable reward instead of ❌ (ineligible). Since this embed only fires on run completion, there is no "pending" state — every marker is final.

Fix:

marker = "✅" if earned else "❌" if losses > (loss_max if loss_max is not None else 99) else "⬜"

The test test_unearned_perfect_bonus_marked_correctly does not catch this — it only asserts "✅" not in bonus_line, which passes for both "⬜" and "❌". A tighter assertion (assert "❌" in bonus_line) is needed.

Security

  • No issues found.

Style & Conventions

Scope: PR bundles three separate features

The diff contains changes from what appear to be three distinct autonomous PRs:

  1. /compare command — cogs/compare.py + tests/test_compare_command.py (this PR)
  2. Gauntlet recap embed — gauntlets.py + tests/test_gauntlet_recap.py (cf. PR #164, still open)
  3. NoResultFound narrowing + reformatting — in_game/gameplay_queries.py (cf. PR #163, still open)

The PR title/body describes only the /compare feature. The gauntlet recap and NoResultFound changes are undisclosed. PRs #163 and #164 are still open with overlapping content, so merging this would cause conflicts.

cogs/compare.py — no issues

The /compare implementation is clean:

  • Arrow semantics are correct; grade_field path in _delta_arrow ignores higher_is_better (so the False in _BATTER_STATS for bunting/hit_and_run is harmless though misleading in comment).
  • Double type-gate (command level + CompareMismatchError) is intentional and correctly annotated.
  • _fetch_player_by_name with limit=1 is fine given autocomplete.
  • player.get("player_id") or player.get("id") safely handles both field name variants.

Suggestions

  • Tighten test_unearned_perfect_bonus_marked_correctly to assert "❌" in bonus_line (not just absence of ✅) regardless of the bug fix, so the test actually pins the intended behavior.

Verdict: REQUEST_CHANGES

One correctness bug: (loss_max or 99) in gauntlets.py:build_gauntlet_recap_embed produces incorrect "⬜" markers for unearned perfect-run rewards. Fix is a one-liner. The test suite does not catch it.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/compare.py` (added — 425 lines, /compare cog) - `gauntlets.py` (modified — gauntlet recap embed) - `in_game/gameplay_queries.py` (modified — NoResultFound narrowing + reformatting) - `paperdynasty.py` (modified — cog registration) - `tests/test_compare_command.py` (added) - `tests/test_gauntlet_recap.py` (added) ### Findings #### Correctness **Bug: `(loss_max or 99)` falsy-zero trap — `gauntlets.py:build_gauntlet_recap_embed`** ```python marker = "✅" if earned else "❌" if losses > (loss_max or 99) else "⬜" ``` When `loss_max == 0` (perfect-run-only reward), Python evaluates `0 or 99 == 99`. So for any completed run with losses (e.g. 10-1), the marker for the 10-0 reward evaluates as `"❌" if 1 > 99` → `False` → `"⬜"`. A player who went 10-1 sees **⬜** (pending/not yet reached) for an unearnable reward instead of **❌** (ineligible). Since this embed only fires on run completion, there is no "pending" state — every marker is final. Fix: ```python marker = "✅" if earned else "❌" if losses > (loss_max if loss_max is not None else 99) else "⬜" ``` The test `test_unearned_perfect_bonus_marked_correctly` does not catch this — it only asserts `"✅" not in bonus_line`, which passes for both "⬜" and "❌". A tighter assertion (`assert "❌" in bonus_line`) is needed. #### Security - No issues found. #### Style & Conventions **Scope: PR bundles three separate features** The diff contains changes from what appear to be three distinct autonomous PRs: 1. `/compare` command — `cogs/compare.py` + `tests/test_compare_command.py` (this PR) 2. Gauntlet recap embed — `gauntlets.py` + `tests/test_gauntlet_recap.py` (cf. PR #164, still open) 3. NoResultFound narrowing + reformatting — `in_game/gameplay_queries.py` (cf. PR #163, still open) The PR title/body describes only the /compare feature. The gauntlet recap and NoResultFound changes are undisclosed. PRs #163 and #164 are still open with overlapping content, so merging this would cause conflicts. **`cogs/compare.py` — no issues** The /compare implementation is clean: - Arrow semantics are correct; `grade_field` path in `_delta_arrow` ignores `higher_is_better` (so the `False` in `_BATTER_STATS` for bunting/hit_and_run is harmless though misleading in comment). - Double type-gate (command level + `CompareMismatchError`) is intentional and correctly annotated. - `_fetch_player_by_name` with `limit=1` is fine given autocomplete. - `player.get("player_id") or player.get("id")` safely handles both field name variants. #### Suggestions - Tighten `test_unearned_perfect_bonus_marked_correctly` to `assert "❌" in bonus_line` (not just absence of ✅) regardless of the bug fix, so the test actually pins the intended behavior. ### Verdict: REQUEST_CHANGES One correctness bug: `(loss_max or 99)` in `gauntlets.py:build_gauntlet_recap_embed` produces incorrect "⬜" markers for unearned perfect-run rewards. Fix is a one-liner. The test suite does not catch it. --- *Automated review by Claude PR Reviewer*
Claude added
ai-changes-requested
 and removed
ai-reviewing
 labels 2026-04-10 16:56:56 +00:00
Claude added the
ai-working
 label 2026-04-10 17:30:50 +00:00
cal added 1 commit 2026-04-10 17:32:17 +00:00
fix(gauntlet): fix loss_max=0 falsy-zero trap in recap marker logic
All checks were successful
Ruff Lint / lint (pull_request) Successful in 19s
Details
cc72827dad 
`(loss_max or 99)` treats `loss_max=0` as 99, so 10-1 runs showed ⬜
instead of ❌ for perfect-run rewards. Fix uses explicit None check.
Tighten test to assert ❌ presence rather than just absence of ✅.

Addresses review feedback on PR #165.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude commented 2026-04-10 17:32:29 +00:00
Collaborator
Copy Link

Addressed review feedback (cc72827):

Bug fix — gauntlets.py:build_gauntlet_recap_embed

  • Changed (loss_max or 99) → (loss_max if loss_max is not None else 99) to avoid the Python falsy-zero trap. A loss_max=0 (perfect-run reward) was being treated as 99, so a 10-1 run would show ⬜ instead of ❌ for the 10-0 bonus.

Test fix — tests/test_gauntlet_recap.py:test_unearned_perfect_bonus_marked_correctly

  • Tightened assertion from assert "✅" not in bonus_line to assert "❌" in bonus_line. The old assertion passed for both ⬜ and ❌, so it didn't catch the bug.

Scope note: The review flagged that this PR bundles changes from three separate features (compare, gauntlet recap, NoResultFound narrowing). Since PRs #163 and #164 are still open with overlapping content, this creates merge-conflict risk. I've left the scope as-is — resolving that would require a rebase/cherry-pick strategy that should be decided by the author rather than automated feedback addressing.

Addressed review feedback (cc72827): **Bug fix — `gauntlets.py:build_gauntlet_recap_embed`** - Changed `(loss_max or 99)` → `(loss_max if loss_max is not None else 99)` to avoid the Python falsy-zero trap. A `loss_max=0` (perfect-run reward) was being treated as 99, so a 10-1 run would show ⬜ instead of ❌ for the 10-0 bonus. **Test fix — `tests/test_gauntlet_recap.py:test_unearned_perfect_bonus_marked_correctly`** - Tightened assertion from `assert "✅" not in bonus_line` to `assert "❌" in bonus_line`. The old assertion passed for both ⬜ and ❌, so it didn't catch the bug. **Scope note**: The review flagged that this PR bundles changes from three separate features (compare, gauntlet recap, NoResultFound narrowing). Since PRs #163 and #164 are still open with overlapping content, this creates merge-conflict risk. I've left the scope as-is — resolving that would require a rebase/cherry-pick strategy that should be decided by the author rather than automated feedback addressing.
Claude removed the
ai-working
ai-changes-requested
 labels 2026-04-10 17:32:38 +00:00
Claude added the
ai-reviewing
 label 2026-04-10 18:19:24 +00:00
Claude reviewed 2026-04-10 18:22:27 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • cogs/compare.py (added)
  • gauntlets.py (modified — gauntlet recap embed)
  • in_game/gameplay_queries.py (modified — NoResultFound + reformat)
  • paperdynasty.py (modified — cog registration)
  • tests/test_compare_command.py (added)
  • tests/test_gauntlet_recap.py (added)

Findings

Correctness

  • No issues found. The loss_max bug previously flagged in this PR's earlier iteration ((loss_max or 99) treating loss_max=0 as 99) is now correctly written as (loss_max if loss_max is not None else 99). The corresponding test (test_unearned_perfect_bonus_marked_correctly) has also been strengthened to assert "❌" in bonus_line, not just absence of "✅" — both the bug and the test gap are resolved.
  • NoResultFound import in gameplay_queries.py is used at 10 call sites — not unused.
  • Arrow logic for grade fields (bunting, hit_and_run): _delta_arrow correctly ignores higher_is_better when grade_field=True, using index-based comparison (A=0 wins over B=1). Setting higher_is_better=False on those stat rows is effectively dead code but documents intent — harmless.
  • Double type-gate in compare_command (pre-fetch check + CompareMismatchError catch) is intentional defence-in-depth; the "should not reach here" log confirms the author is aware.

Security

  • No issues found. User input (card1, card2) is passed as query params to db_get (HTTP API), not directly into SQL. Role gate (PD_PLAYERS_ROLE_NAME) correctly guards the command.

Style & Conventions

  • No issues found. The compare cog follows existing cog patterns (role check, defer, edit_original_response). Pure builder function is testable without Discord state.

Suggestions

  • Scope (coordination risk, not a blocker): This PR bundles three separate concerns: /compare (its title), the gauntlet recap embed (also the subject of open PR #164), and gameplay_queries.py NoResultFound narrowing (also the subject of open PR #163). Merging this while #163 or #164 are still open will cause merge conflicts or duplicate changes on those branches. Recommend closing #163 and #164 in favour of this PR, or rebasing them off this branch once it lands.
  • _build_card_data — None player_id edge case: pid = player.get("player_id") or player.get("id") falls back to None if neither key is present, which would pass None to the battingcards/pitchingcards API. Practically impossible given valid API responses, but worth a if not pid: return {"player": player, "battingcard": None, "pitchingcard": None} guard.

Verdict: COMMENT

Compare implementation is correct, well-tested, and follows project conventions. The gauntlet recap code bundled here is actually a fixed version of what's blocking PR #164. No correctness issues. Main action item is coordinating closure of #163 and #164 to avoid conflicts.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/compare.py` (added) - `gauntlets.py` (modified — gauntlet recap embed) - `in_game/gameplay_queries.py` (modified — NoResultFound + reformat) - `paperdynasty.py` (modified — cog registration) - `tests/test_compare_command.py` (added) - `tests/test_gauntlet_recap.py` (added) ### Findings #### Correctness - **No issues found.** The `loss_max` bug previously flagged in this PR's earlier iteration (`(loss_max or 99)` treating `loss_max=0` as `99`) is now correctly written as `(loss_max if loss_max is not None else 99)`. The corresponding test (`test_unearned_perfect_bonus_marked_correctly`) has also been strengthened to assert `"❌" in bonus_line`, not just absence of `"✅"` — both the bug and the test gap are resolved. - `NoResultFound` import in `gameplay_queries.py` is used at 10 call sites — not unused. - Arrow logic for grade fields (`bunting`, `hit_and_run`): `_delta_arrow` correctly ignores `higher_is_better` when `grade_field=True`, using index-based comparison (A=0 wins over B=1). Setting `higher_is_better=False` on those stat rows is effectively dead code but documents intent — harmless. - Double type-gate in `compare_command` (pre-fetch check + `CompareMismatchError` catch) is intentional defence-in-depth; the "should not reach here" log confirms the author is aware. #### Security - No issues found. User input (`card1`, `card2`) is passed as query params to `db_get` (HTTP API), not directly into SQL. Role gate (`PD_PLAYERS_ROLE_NAME`) correctly guards the command. #### Style & Conventions - No issues found. The compare cog follows existing cog patterns (role check, `defer`, `edit_original_response`). Pure builder function is testable without Discord state. #### Suggestions - **Scope (coordination risk, not a blocker):** This PR bundles three separate concerns: `/compare` (its title), the gauntlet recap embed (also the subject of open PR #164), and `gameplay_queries.py` NoResultFound narrowing (also the subject of open PR #163). Merging this while #163 or #164 are still open will cause merge conflicts or duplicate changes on those branches. Recommend closing #163 and #164 in favour of this PR, or rebasing them off this branch once it lands. - **`_build_card_data` — None player_id edge case:** `pid = player.get("player_id") or player.get("id")` falls back to `None` if neither key is present, which would pass `None` to the battingcards/pitchingcards API. Practically impossible given valid API responses, but worth a `if not pid: return {"player": player, "battingcard": None, "pitchingcard": None}` guard. ### Verdict: COMMENT Compare implementation is correct, well-tested, and follows project conventions. The gauntlet recap code bundled here is actually a fixed version of what's blocking PR #164. No correctness issues. Main action item is coordinating closure of #163 and #164 to avoid conflicts. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-10 18:23:06 +00:00
Claude removed the
ai-reviewed
 label 2026-04-11 19:23:46 +00:00
claude-reviewer added the
ai-reviewing
 label 2026-04-11 21:23:14 +00:00
claude-reviewer approved these changes 2026-04-11 21:26:52 +00:00
claude-reviewer left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • cogs/compare.py (added — 425 lines, /compare command + embed builder)
  • tests/test_compare_command.py (added — 395 lines, 30 tests)
  • gauntlets.py (modified — gauntlet recap embed, same as PR #164)
  • in_game/gameplay_queries.py (modified — NoResultFound narrowing + reformat, same as PR #163)
  • paperdynasty.py (modified — cog registration)

Findings

Correctness

No issues found.

  • _delta_arrow logic is correct across all three paths (numeric higher/lower, grade fields, tied/missing). The grade_field=True path correctly short-circuits before higher_is_better is consulted — grade semantics are self-contained.
  • Grade indexing is correct: GRADE_ORDER.index("A") = 0; ▲ when n2 < n1 (card2 has a lower/better grade index). Verified against test_grade_field_a_beats_b.
  • rarity_value comparison uses rarity.value (int) while display uses rarity.name (string) — intentional and correct.
  • Type-gate fires at two levels (command level on raw player dicts, embed builder level on card data dicts). The CompareMismatchError branch in the command is correctly guarded with a comment noting it should not be reachable.
  • _fetch_player_by_name uses players/search endpoint confirmed to exist (utilities/autocomplete.py:79) and return {"players": [...]} — response key matches.
  • _build_card_data uses player.get("player_id") or player.get("id") — acceptable or-chain fallback given player IDs are non-zero.
  • gauntlets.py / gameplay_queries.py changes unchanged from prior approved reviews of #163 and #164.

Security

No issues found. All API calls go through parameterised db_get. No user input reaches raw queries or commands.

Style & Conventions

No issues. Follows project patterns: db_get, player_autocomplete, @app_commands.checks.has_any_role, try/except with logger.error, await interaction.response.defer().

Suggestions

  1. Scope: close #163 and #164 before merging this PR. This PR still bundles all three features. #163 and #164 are open. If either merges to main first, this branch will need a rebase on gameplay_queries.py / gauntlets.py. Safest path: merge #163 → merge #164 → rebase this branch → merge this PR. Alternatively, close #163 and #164 since their changes are already included here.

  2. _BATTER_STATS tuple comment reads (label, key_in_card, key_in_player, higher_is_better) but the third element is source (the data source: "player", "battingcard", "pitchingcard"), not a key within the player dict. Minor comment inaccuracy, not a code issue.

  3. higher_is_better=False on grade-field entries ("bunting", "hit_and_run") is a no-op — the grade_field=True path ignores it entirely. The values could be anything without affecting output. Consider True or a note in the comment to avoid future confusion.

Verdict: APPROVED

Clean implementation. Arrow semantics correct, edge cases covered, players/search endpoint confirmed. The only action item before merge is resolving the scope overlap with #163 and #164.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/compare.py` (added — 425 lines, /compare command + embed builder) - `tests/test_compare_command.py` (added — 395 lines, 30 tests) - `gauntlets.py` (modified — gauntlet recap embed, same as PR #164) - `in_game/gameplay_queries.py` (modified — NoResultFound narrowing + reformat, same as PR #163) - `paperdynasty.py` (modified — cog registration) ### Findings #### Correctness No issues found. - `_delta_arrow` logic is correct across all three paths (numeric higher/lower, grade fields, tied/missing). The `grade_field=True` path correctly short-circuits before `higher_is_better` is consulted — grade semantics are self-contained. - Grade indexing is correct: `GRADE_ORDER.index("A") = 0`; `▲` when `n2 < n1` (card2 has a lower/better grade index). Verified against `test_grade_field_a_beats_b`. - `rarity_value` comparison uses `rarity.value` (int) while display uses `rarity.name` (string) — intentional and correct. - Type-gate fires at two levels (command level on raw player dicts, embed builder level on card data dicts). The `CompareMismatchError` branch in the command is correctly guarded with a comment noting it should not be reachable. - `_fetch_player_by_name` uses `players/search` endpoint confirmed to exist (`utilities/autocomplete.py:79`) and return `{"players": [...]}` — response key matches. - `_build_card_data` uses `player.get("player_id") or player.get("id")` — acceptable `or`-chain fallback given player IDs are non-zero. - `gauntlets.py` / `gameplay_queries.py` changes unchanged from prior approved reviews of #163 and #164. #### Security No issues found. All API calls go through parameterised `db_get`. No user input reaches raw queries or commands. #### Style & Conventions No issues. Follows project patterns: `db_get`, `player_autocomplete`, `@app_commands.checks.has_any_role`, try/except with `logger.error`, `await interaction.response.defer()`. #### Suggestions 1. **Scope: close #163 and #164 before merging this PR.** This PR still bundles all three features. #163 and #164 are open. If either merges to `main` first, this branch will need a rebase on `gameplay_queries.py` / `gauntlets.py`. Safest path: merge #163 → merge #164 → rebase this branch → merge this PR. Alternatively, close #163 and #164 since their changes are already included here. 2. `_BATTER_STATS` tuple comment reads `(label, key_in_card, key_in_player, higher_is_better)` but the third element is `source` (the data source: `"player"`, `"battingcard"`, `"pitchingcard"`), not a key within the player dict. Minor comment inaccuracy, not a code issue. 3. `higher_is_better=False` on grade-field entries (`"bunting"`, `"hit_and_run"`) is a no-op — the `grade_field=True` path ignores it entirely. The values could be anything without affecting output. Consider `True` or a note in the comment to avoid future confusion. ### Verdict: APPROVED Clean implementation. Arrow semantics correct, edge cases covered, `players/search` endpoint confirmed. The only action item before merge is resolving the scope overlap with #163 and #164. --- *Automated review by Claude PR Reviewer*
claude-reviewer added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-11 21:27:18 +00:00
All checks were successful
Ruff Lint / lint (pull_request) Successful in 19s
Details
This pull request has changes requested by an official reviewer.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin autonomous/feat-compare-command:autonomous/feat-compare-command
git checkout autonomous/feat-compare-command
Sign in to join this conversation.
Reviewers
No reviewers
Claude
claude-reviewer
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working

   
autonomous

PR created by the autonomous improvement pipeline

  
bug

   
enhancement

   
feature

New feature proposal

  
in-queue

Approved for development

  
performance

   
security

   
size:M

Medium PR (100-500 lines)

  
size:S

Small PR (< 100 lines)

  
tech-debt

   
tests

Test coverage and test quality

  
todo

   
type:feature

Feature work

  
type:stability

Stability fix (tests, dead code, bugs)

No Label
ai-changes-requested
ai-failed
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
autonomous
bug
enhancement
feature
in-queue
performance
security
size:M
size:S
tech-debt
tests
todo
type:feature
type:stability
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-discord#165
No description provided.
Delete Branch "autonomous/feat-compare-command"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?