feat: rarity ladder utility for safe next-rarity lookups (#59) #64

Claude · 2026-04-08T12:32:06Z

Claude commented

2026-04-08 12:32:06 +00:00

Closes #59

Summary

Adds RARITY_LADDER, rarity_is_downgrade(), and next_rarity() to rarity_thresholds.py.

RARITY_LADDER — ordered list [5, 4, 3, 2, 1, 99] (Common → Bronze → Silver → Gold → Diamond → HoF). Canonical ordering for all tier comparisons.
next_rarity(current_id) — returns the next more-prestigious rarity ID, or None at HoF or for unknown IDs. Prevents the naive rarity_id + 1 bug (100 for HoF, 0 for Diamond).
rarity_is_downgrade(current_id, new_id) — returns True if new_id is less prestigious than current_id. Used by the T4 rarity guard (card-creation#57).

Files Changed

rarity_thresholds.py — +53 lines (ladder constant + two helper functions)

Tests

No test suite in this repo. Logic verified by reading the implementation: RARITY_LADDER.index() lookups handle all edge cases cleanly, and None is returned at both ends (HoF max, unknown ID).

Closes #59 ## Summary Adds `RARITY_LADDER`, `rarity_is_downgrade()`, and `next_rarity()` to `rarity_thresholds.py`. - **`RARITY_LADDER`** — ordered list `[5, 4, 3, 2, 1, 99]` (Common → Bronze → Silver → Gold → Diamond → HoF). Canonical ordering for all tier comparisons. - **`next_rarity(current_id)`** — returns the next more-prestigious rarity ID, or `None` at HoF or for unknown IDs. Prevents the naive `rarity_id + 1` bug (100 for HoF, 0 for Diamond). - **`rarity_is_downgrade(current_id, new_id)`** — returns `True` if `new_id` is less prestigious than `current_id`. Used by the T4 rarity guard (card-creation#57). ## Files Changed - `rarity_thresholds.py` — +53 lines (ladder constant + two helper functions) ## Tests No test suite in this repo. Logic verified by reading the implementation: `RARITY_LADDER.index()` lookups handle all edge cases cleanly, and `None` is returned at both ends (HoF max, unknown ID).

Claude added 1 commit 2026-04-08 12:32:07 +00:00

feat: add RARITY_LADDER, rarity_is_downgrade, and next_rarity helpers (#59 ) 1d593a91a6

Closes #59

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

Claude referenced this pull request

2026-04-08 12:32:15 +00:00

feat: rarity ladder utility for safe next-rarity lookups #59

Claude added the

ai-reviewing

label 2026-04-08 12:45:03 +00:00

Claude removed the

ai-reviewing

label 2026-04-08 12:50:04 +00:00

Claude added the

ai-reviewing

label 2026-04-08 13:17:45 +00:00

Claude removed the

ai-reviewing

label 2026-04-08 13:22:47 +00:00

Claude added the

ai-reviewing

label 2026-04-08 13:48:25 +00:00

Claude reviewed 2026-04-08 13:51:25 +00:00

Claude left a comment

AI Code Review

Files Reviewed

rarity_thresholds.py (modified)

Findings

Correctness

Merge conflict — PR is not mergeable. RARITY_LADDER and rarity_is_downgrade were already added to main as part of PR #63 (T4 rarity guard fix, commit aaa2eaa). The current main has both at lines 149–168. This PR's branch was created before that merge, so it conflicts on those exact lines. Gitea confirms "mergeable": false. The branch needs to be rebased onto current main; after rebase, only next_rarity will be a new addition.

After rebasing, the RARITY_LADDER already on main lacks the list[int] type annotation — the PR's version (RARITY_LADDER: list[int] = [...]) is preferable and should be kept.
rarity_is_downgrade unknown-new_rarity_id behaviour not documented. The docstring says "Unknown IDs are treated as position 0 (worst), so an unknown current rarity will never trigger a downgrade guard." It omits what happens when new_rarity_id is unknown (also returns False). A one-sentence addition would close the gap (e.g. "An unknown new_rarity_id also returns False."). The behaviour itself is fine — it avoids false positives when the system produces an unexpected ID — but the omission makes the contract ambiguous.

Security

No issues found.

Style & Conventions

No issues found. next_rarity docstring with examples is clear and follows existing patterns.

Suggestions

next_rarity is not yet called anywhere in the codebase. Correct for a utility being added ahead of use. No action needed.

Verdict: REQUEST_CHANGES (posted as COMMENT — Gitea blocks self-review)

The next_rarity logic is correct. Two issues block merge:

Branch has a merge conflict with main (introduced by PR #63) — rebase required.
rarity_is_downgrade docstring should document the unknown-new_rarity_id return path.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `rarity_thresholds.py` (modified) ### Findings #### Correctness - **Merge conflict — PR is not mergeable.** `RARITY_LADDER` and `rarity_is_downgrade` were already added to `main` as part of PR #63 (T4 rarity guard fix, commit `aaa2eaa`). The current `main` has both at lines 149–168. This PR's branch was created before that merge, so it conflicts on those exact lines. Gitea confirms `"mergeable": false`. The branch needs to be rebased onto current `main`; after rebase, only `next_rarity` will be a new addition. After rebasing, the `RARITY_LADDER` already on main lacks the `list[int]` type annotation — the PR's version (`RARITY_LADDER: list[int] = [...]`) is preferable and should be kept. - **`rarity_is_downgrade` unknown-`new_rarity_id` behaviour not documented.** The docstring says "Unknown IDs are treated as position 0 (worst), so an unknown **current** rarity will never trigger a downgrade guard." It omits what happens when `new_rarity_id` is unknown (also returns `False`). A one-sentence addition would close the gap (e.g. "An unknown `new_rarity_id` also returns False."). The behaviour itself is fine — it avoids false positives when the system produces an unexpected ID — but the omission makes the contract ambiguous. #### Security - No issues found. #### Style & Conventions - No issues found. `next_rarity` docstring with examples is clear and follows existing patterns. #### Suggestions - `next_rarity` is not yet called anywhere in the codebase. Correct for a utility being added ahead of use. No action needed. ### Verdict: REQUEST_CHANGES (posted as COMMENT — Gitea blocks self-review) The `next_rarity` logic is correct. Two issues block merge: 1. Branch has a merge conflict with `main` (introduced by PR #63) — rebase required. 2. `rarity_is_downgrade` docstring should document the unknown-`new_rarity_id` return path. --- *Automated review by Claude PR Reviewer*

Claude added

ai-changes-requested

and removed

ai-reviewing

labels 2026-04-08 13:52:00 +00:00

Claude added the

ai-working

label 2026-04-08 14:00:50 +00:00

cal force-pushed issue/59-feat-rarity-ladder-utility-for-safe-next-rarity-lo from 1d593a91a6 to feedfe951f

2026-04-08 14:02:21 +00:00

Compare

Claude commented

2026-04-08 14:02:28 +00:00

Addressed review feedback:

Rebased onto current main — resolved merge conflict with PR #63. After rebase, only next_rarity is a new addition; RARITY_LADDER and rarity_is_downgrade already existed on main. Kept the list[int] type annotation and expanded multi-line format for RARITY_LADDER (preferred over the untyped one-liner that landed via #63).
Updated rarity_is_downgrade docstring — added "An unknown new_rarity_id also returns False." to close the contract gap noted in the review.

Addressed review feedback: 1. **Rebased onto current `main`** — resolved merge conflict with PR #63. After rebase, only `next_rarity` is a new addition; `RARITY_LADDER` and `rarity_is_downgrade` already existed on `main`. Kept the `list[int]` type annotation and expanded multi-line format for `RARITY_LADDER` (preferred over the untyped one-liner that landed via #63). 2. **Updated `rarity_is_downgrade` docstring** — added "An unknown `new_rarity_id` also returns False." to close the contract gap noted in the review.

Claude removed the

ai-working

ai-changes-requested

labels 2026-04-08 14:02:41 +00:00

Claude added the

ai-reviewing

label 2026-04-08 14:15:50 +00:00

Claude reviewed 2026-04-08 14:16:55 +00:00

Claude left a comment

AI Code Review

Files Reviewed

rarity_thresholds.py (modified)

Findings

Correctness

No issues found. next_rarity() logic verified against all inputs:

Call	pos	Result
`next_rarity(5)`	0	`4` (Bronze) ✓
`next_rarity(4)`	1	`3` (Silver) ✓
`next_rarity(3)`	2	`2` (Gold) ✓
`next_rarity(2)`	3	`1` (Diamond) ✓
`next_rarity(1)`	4	`99` (HoF) ✓
`next_rarity(99)`	5 == `len(RARITY_LADDER)-1`	`None` ✓
`next_rarity(unknown)`	`ValueError`	`None` ✓

All docstring examples match implementation. The rarity_is_downgrade docstring addition ("An unknown new_rarity_id also returns False") accurately describes the existing try/except behaviour.

Prior review requested a rebase on top of PR #63 — confirmed resolved. Base is now f3da587 (PR #63 merge) and mergeable: true.

Security

No issues found.

Style & Conventions

list[int] type annotation and int | None return type are consistent with existing annotations in the file.
Multiline RARITY_LADDER format is clean and readable.

Suggestions

None.

Verdict: COMMENT

LGTM. Rebase resolved the prior conflict; next_rarity() is correct and handles all edge cases. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `rarity_thresholds.py` (modified) ### Findings #### Correctness No issues found. `next_rarity()` logic verified against all inputs: | Call | pos | Result | |------|-----|--------| | `next_rarity(5)` | 0 | `4` (Bronze) ✓ | | `next_rarity(4)` | 1 | `3` (Silver) ✓ | | `next_rarity(3)` | 2 | `2` (Gold) ✓ | | `next_rarity(2)` | 3 | `1` (Diamond) ✓ | | `next_rarity(1)` | 4 | `99` (HoF) ✓ | | `next_rarity(99)` | 5 == `len(RARITY_LADDER)-1` | `None` ✓ | | `next_rarity(unknown)` | `ValueError` | `None` ✓ | All docstring examples match implementation. The `rarity_is_downgrade` docstring addition ("An unknown new_rarity_id also returns False") accurately describes the existing try/except behaviour. Prior review requested a rebase on top of PR #63 — confirmed resolved. Base is now `f3da587` (PR #63 merge) and `mergeable: true`. #### Security No issues found. #### Style & Conventions - `list[int]` type annotation and `int | None` return type are consistent with existing annotations in the file. - Multiline RARITY_LADDER format is clean and readable. #### Suggestions None. ### Verdict: COMMENT LGTM. Rebase resolved the prior conflict; `next_rarity()` is correct and handles all edge cases. Ready to merge. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-08 14:17:23 +00:00

cal approved these changes 2026-04-08 14:26:09 +00:00

cal left a comment

Approved via pd-pr

cal merged commit 802979a851 into main

2026-04-08 14:26:25 +00:00

cal referenced this issue from a commit

2026-04-08 14:26:26 +00:00